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

Exclude jedis instrumentation from Indy-automigration #9675

Merged
merged 2 commits into from
Oct 16, 2023

Conversation

JonasKunz
Copy link
Contributor

@JonasKunz JonasKunz commented Oct 13, 2023

Looks like #9654 uncovered some new indy test failures. Those suddenly appeared in #9565 after a rebase.

I think those exceptions come from the fact that some jedis instrumentations are applied even though they shouldn't, likely because the muzzle check is not active yet.

After some more looking I think the reason is actually that the Advice passes a JedisRequestContext from enter to exit. This type is then present on the stack of the instrumented method, causing it to fail because JedisRequestContext is not injected anymore.

Please add the test Indy label to this PR to verify it actually fixes the build.

@JonasKunz JonasKunz requested a review from a team October 13, 2023 12:53
Copy link
Contributor

@laurit laurit left a comment

Choose a reason for hiding this comment

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

There are some other tests where indy is disabled for the same reason. It could be fixed by returning Object instead and casting in the exit advice, but that isn't too nice. I think this could probably be worked around by changing bytebuddy.

@laurit laurit merged commit 6b0f479 into open-telemetry:main Oct 16, 2023
50 checks passed
@JonasKunz JonasKunz deleted the jedis-indy-exclusion branch October 16, 2023 12:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants