Skip to content

Local context is overwritten by Observation instrumentation #760

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

Closed
koenpunt opened this issue Jul 20, 2023 · 7 comments
Closed

Local context is overwritten by Observation instrumentation #760

koenpunt opened this issue Jul 20, 2023 · 7 comments
Assignees
Labels
status: superseded Issue is superseded by another

Comments

@koenpunt
Copy link
Contributor

We haven't been able to update to a newer version than 1.1.3 because unfortunately every time new bugs are introduced.

The last thing that was broken was #742, now that's fixed and out with the 3.1.2 boot-starter. However, now apparently using local context no longer works.

We have the following in multiple places in our application:

DataFetcherResult.newResult<T>()
        .data(data)
        .localContext(GraphQLContext.of(contextMap))
        .build()

But now all that is found in the local context in child resolver functions is the micrometer.observation key.

I understand it's not intentional, but it is quite annoying that fixes are released with other changes that cause new issues.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Jul 20, 2023
@koenpunt
Copy link
Contributor Author

After some more testing I see this actually broke between 1.1.3 and 1.1.4

@koenpunt
Copy link
Contributor Author

I think the problem lies here, where a new result is created without copying the localContext from the environment:

return DataFetcherResult.newResult()
.data(value)
.localContext(GraphQLContext.newContext().of(ObservationThreadLocalAccessor.KEY, dataFetcherObservation).build())
.build();

@bclozel
Copy link
Member

bclozel commented Jul 20, 2023

Closing in favor of #761

@bclozel bclozel closed this as not planned Won't fix, can't repro, duplicate, stale Jul 20, 2023
@bclozel bclozel added status: superseded Issue is superseded by another and removed status: waiting-for-triage An issue we've not yet triaged labels Jul 20, 2023
@bclozel bclozel changed the title Providing local context using DataFetcherResult no longer works 1.1.3 => 1.2.2 Local context is overwritten by Observation instrumentation Aug 16, 2023
@bclozel
Copy link
Member

bclozel commented Sep 14, 2023

@koenpunt we have fixed quite a few important issues lately. Can you check that the current SNAPSHOTs are working well with your applications? We are going to release maintenance versions for 1.1.x and 1.2.x in a few days.
Thanks!

@koenpunt
Copy link
Contributor Author

We're currently running the 1.2.3 snapshot which works for us 👍

@ABINTC
Copy link

ABINTC commented Nov 23, 2023

@bclozel I still see this issue in 1.2.4 ! 😔

@bclozel
Copy link
Member

bclozel commented Nov 23, 2023

@ABINTC please create a new issue with a minimal, sample application reproducing the problem and I'll have a look. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: superseded Issue is superseded by another
Projects
None yet
4 participants