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

Inherit local context in observation instrumentation instead of overwriting it #761

Closed
wants to merge 2 commits into from

Conversation

koenpunt
Copy link
Contributor

@koenpunt koenpunt commented Jul 20, 2023

When the datafetcher did not return a DataFetcherResult, the existing context was discarded and a new context was created.

Now when there's no DataFetcherResult returned, we first check if there's local context available in the environment, and if so, the key is added to it. Otherwise we still create a new context.

Should fix #760

@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

I just tried a build with these changes in our application and now it works again.

@bclozel bclozel added type: bug A general bug in: data Issues related to working with data and removed status: waiting-for-triage An issue we've not yet triaged labels Jul 20, 2023
@bclozel bclozel self-assigned this Jul 20, 2023
@bclozel bclozel added this to the 1.2.3 milestone Aug 3, 2023
@bclozel bclozel added the for: backport-to-1.2.x Marks an issue as a candidate for backport to 1.2.x label Aug 16, 2023
bclozel pushed a commit that referenced this pull request Aug 16, 2023
Prior to this commit, a `DataFetcher` instrumented by the
`GraphQlObservationInstrumentation` would incorrectly overwrite the
local context when:

* the `DataFetcher` returns a value object (i.e. not a
  `DataFetcherResult`)
* the given `DatFetchingEnvironment` has an existing local context
with values

For this case, the instrumentation would create a new local context but
would not inherit from the existing local context.

See gh-761
@bclozel bclozel closed this in 94ba9fe Aug 16, 2023
@bclozel bclozel added status: backported An issue that has been backported to maintenance branches and removed for: backport-to-1.2.x Marks an issue as a candidate for backport to 1.2.x labels Aug 16, 2023
@koenpunt koenpunt deleted the inherit-localcontext branch August 21, 2023 12:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: data Issues related to working with data status: backported An issue that has been backported to maintenance branches type: bug A general bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Local context is overwritten by Observation instrumentation
3 participants