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

Added support for insecure connection with Sentry #3446

Merged
merged 7 commits into from
Jun 10, 2021

Conversation

maksim77
Copy link
Contributor

Description:
Added an option that disables ssl certificate verification.
If Sentry is deployed in a private cloud and its ssl certificate is not valid (from a global point of view), then traces will simply not be sent. What's especially bad is that it happens quietly. There is no error anywhere in the logs.

Testing:
The package tests have not changed and are running successfully

Documentation:
Updated the readme

@maksim77 maksim77 requested a review from a team as a code owner May 21, 2021 13:10
@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented May 21, 2021

CLA Signed

The committers are authorized under a signed CLA.

@project-bot project-bot bot added this to In progress in Collector May 21, 2021
@bogdandrutu
Copy link
Member

@AbhiPrasad please review

Copy link
Member

@AbhiPrasad AbhiPrasad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your contribution. I have a remark on naming, and then a broader question on using transports with different exporters.

exporter/sentryexporter/README.md Outdated Show resolved Hide resolved
exporter/sentryexporter/sentry_exporter.go Show resolved Hide resolved
Collector automation moved this from In progress to Review in progress May 26, 2021
@maksim77
Copy link
Contributor Author

@AbhiPrasad i can see that the tests failed but clearly not because of the proposed changes. Should I do something about it?

@AbhiPrasad
Copy link
Member

Yeah not sure why the tests are failing. Also, I would rebase on main since #3542 merged in, maybe that will fix them?

Copy link
Member

@AbhiPrasad AbhiPrasad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes are fine from Sentry's side. Thanks again for the patch!

@github-actions
Copy link
Contributor

github-actions bot commented Jun 4, 2021

This PR was marked stale due to lack of activity. It will be closed in 7 days.

@github-actions github-actions bot added the Stale label Jun 4, 2021
@maksim77
Copy link
Contributor Author

maksim77 commented Jun 4, 2021

@bogdandrutu @kbrockhoff What are the next steps?

@github-actions github-actions bot removed the Stale label Jun 5, 2021
Collector automation moved this from Review in progress to Reviewer approved Jun 10, 2021
@bogdandrutu bogdandrutu merged commit 432e340 into open-telemetry:main Jun 10, 2021
Collector automation moved this from Reviewer approved to Done Jun 10, 2021
alexperez52 referenced this pull request in open-o11y/opentelemetry-collector-contrib Aug 18, 2021
Signed-off-by: Bogdan Drutu <bogdandrutu@gmail.com>
mstumpfx pushed a commit to mstumpfx/opentelemetry-collector-contrib that referenced this pull request Aug 31, 2021
* Added support for insecure connection with Sentry

* update README.md

* add doc string to Config struct

* rename insecure configuration flag

* update README.md according to new exporter config

* update example according to new exporter config
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Collector
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

4 participants