Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Gitlab Sentry-Backend DSN Support #506

Closed
AtjonTV opened this issue May 28, 2022 · 3 comments
Closed

Gitlab Sentry-Backend DSN Support #506

AtjonTV opened this issue May 28, 2022 · 3 comments

Comments

@AtjonTV
Copy link
Contributor

AtjonTV commented May 28, 2022

Hello!

Today I got the urge to try Sentry-Elixir with the Gitlab Sentry-backend, but sadly the way this client parses the DSN and creates the endpoint is not compatible with it.

I debugged a little and compared the source with how the Ruby version does it.

From my debugging I have gathered the following diff:

diff --git a/lib/sentry/client.ex b/lib/sentry/client.ex
index 0a002ed..6dc84e6 100644
--- a/lib/sentry/client.ex
+++ b/lib/sentry/client.ex
@@ -257,9 +257,11 @@ defmodule Sentry.Client do
          %URI{userinfo: userinfo, host: host, port: port, path: path, scheme: protocol}
          when is_binary(path) and is_binary(userinfo) <- URI.parse(dsn),
          [public_key, secret_key] <- keys_from_userinfo(userinfo),
-         [_, binary_project_id] <- String.split(path, "/"),
+         uri_path <- String.split(path, "/"),
+         {binary_project_id, uri_path} <- List.pop_at(uri_path, -1),
+         base_path <- Enum.join(uri_path, "/"),
          {project_id, ""} <- Integer.parse(binary_project_id),
-         endpoint <- "#{protocol}://#{host}:#{port}/api/#{project_id}/envelope/" do
+         endpoint <- "#{protocol}://#{host}:#{port}#{base_path}/api/#{project_id}/store/" do
       {endpoint, public_key, secret_key}
     else
       _ ->

With this diff applied the Gitlab Sentry-backend DSN is parsed correctly, and the endpoint url is also constructed correctly.

iex(27)> Debug.get_dsn_debug "https://glet_asdasdasd@gitlab.atvg-studios.atvg.cloud/api/v4/error_tracking/collector/276"          
{"https://gitlab.atvg-studios.atvg.cloud:443/api/v4/error_tracking/collector/api/276/store/", "glet_asdasdasd", nil}

As I am new to Elixir I am not sure if this is the best possible implementation, because of this I am creating this issue instead of a merge request.

I would love to see this fixed in the client and am willing to open a MR if I get feedback on this. Alternatively a maintainer could also just quickly implement that patch.

@mitchellhenke
Copy link
Contributor

A PR/MR would be most welcome!

@AtjonTV
Copy link
Contributor Author

AtjonTV commented May 28, 2022

I have fixed my mess up of changing /envelope to /store and opend a MR: #507

@mitchellhenke
Copy link
Contributor

Thank you!!

Closed by #507

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants