-
-
Notifications
You must be signed in to change notification settings - Fork 444
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
Auto add OpenTelemetryLinkErrorEventProcessor for Spring Boot #2429
Merged
adinauer
merged 11 commits into
main
from
feat/auto-add-otel-event-processor-for-spring-boot
Dec 22, 2022
Merged
Changes from 9 commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
baf8bd5
Add tests for linking errors to otel transactions
adinauer 49efc4c
Add logging to OTEL code
adinauer 183423a
Add more IDs to log statements
adinauer 8b47a99
Add changelog
adinauer e3c8470
Auto add OpenTelemetryLinkErrorEventProcessor for Spring Boot
adinauer 707ea59
Add changelog
adinauer ba730c2
No longer require direct dependency on OTEL SDK in spring boot app wh…
adinauer 319b527
Merge branch 'main' into feat/test-error-otel-transaction-link
adinauer d7813ca
Merge branch 'feat/test-error-otel-transaction-link' into feat/auto-a…
adinauer da36dbf
Merge branch 'main' into feat/auto-add-otel-event-processor-for-sprin…
adinauer 44f9f2e
Merge branch 'main' into feat/auto-add-otel-event-processor-for-sprin…
adinauer File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hm, why did we change it to
implementation
? It's not possible to use this module without otel, right? Or is the assumption that people will just include oursentry-opentelemetry-core
package, but not otel itself?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The intention was to make it easier to use
sentry-opentelemetry-agent
withSENTRY_AUTO_INIT=false
. Currently you need to addsentry-opentelemetry-core
to the dependencies so you can enjoy linking of errors to transactions. WithcompileOnly
you'd also have to include the correct version of the OTEL SDK, withimplementation
it just drags it in. Forsentry-opentelemetry-agent
we exclude it again, which users also have to do, if they don't want to use the agent but have a different OTEL version than we use or their build system can't figure out versions.Another approach (for later) would be to have yet another gradle module, that we can then add to the bootstrap classloader in the agent, thus not requiring the explicit dependency on
sentry-opentelemetry-core
at all, when settingSENTRY_AUTO_INIT=false
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
alright, just beware that
implementation
will override whatever version the users are using (if it's lower than ours). If that's what we want, then I'm good 👍