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

Expose SentryInstrumentation as bean with @ConditionalOnMissingBean for easy configuration #3412

Closed
ooraini opened this issue May 9, 2024 · 5 comments
Assignees
Milestone

Comments

@ooraini
Copy link
Contributor

ooraini commented May 9, 2024

Problem Statement

I trying to use SentryInstrumentation.BeforeSpanCallback with the auto configuration from Sentry, but unfortunately with the way it's set up, it's very hard to override. I'm not sure what is the best solution, perhaps it could be defined as @bean with @ConditionalOnMissingBean. I doubt there will be more than one in the application context.

Solution Brainstorm

No response

@adinauer
Copy link
Member

Thanks for the suggestion @ooraini we should be able to make it a @Bean with @ConditionalOnMissingBean. As a workaround, can you try to exclude the SentryGraphqlConfiguration and write your own until we change the SDK?

@ooraini
Copy link
Contributor Author

ooraini commented May 10, 2024

@adinauer Thanks for the reply

can you try to exclude the SentryGraphqlConfiguration and write your own until we change the SDK?

From what I know(and some research), I don't think you can excluded a @Configuration class, only @AutoConfiguration class can be excluded.

I'll try it out and see, maybe it works.

@jon-mercer
Copy link

Adding to this, I just spent entirely too long running down an issue where graphql.execution.instrumentation.Instrumentation beans other than the SentryInstrumentation were not running at all, including spring-graphql's org.springframework.graphql.observation.GraphQlObservationInstrumentation which we were expecting to produce metrics.

Normally what Spring does is find all Instrumentation beans, put them into a single graphql.execution.instrumentation.ChainedInstrumentation and puts that into graphql-java's graphql.GraphQL instance.

I determined the root cause to be that io.sentry.spring.graphql.SentryGraphqlConfiguration is replacing that single ChainedInstrumentation with just SentryInstrumentation, removing all other Instrumentation beans.

If SentryInstrumentation were a @Bean instead of using a GraphQlSourceBuilderCustomizer, this would not be an issue.

So, +1 to the suggestion of making SentryInstrumentation a @Bean instead

@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 3 May 15, 2024
@adinauer
Copy link
Member

Thanks for the insight @jon-mercer, we'd like to tackle this for the upcoming v8 of the Java SDK.

@adinauer
Copy link
Member

@ooraini and @jon-mercer we've just released 8.0.0-beta.1 of the Java SDK which defines SentryInstrumentation as a bean instead of using GraphQlSourceBuilderCustomizer.

It is now also possible to expose a bean of type SentryGraphqlInstrumentation.BeforeSpanCallback which should then be used automatically.

Please note if you're using graphql-java v22 or newer, there's now a new Sentry dependency sentry-graphql-22 which supports it. Some imports have to be changed from io.sentry.graphql to io.sentry.graphql22.

If you decide to give it a try, feedback would be great :-)

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

No branches or pull requests

3 participants