fix: skip instrumenting dalli 4.2.0#1982
Open
hannahramadan wants to merge 2 commits intoopen-telemetry:mainfrom
Open
fix: skip instrumenting dalli 4.2.0#1982hannahramadan wants to merge 2 commits intoopen-telemetry:mainfrom
hannahramadan wants to merge 2 commits intoopen-telemetry:mainfrom
Conversation
hannahramadan
requested review from
a team,
ahayworth,
arielvalentin,
dazuma,
ericmustin,
fbogsany,
kaylareopelle,
mwear,
plantfansam,
robbkidd,
robertlaurin,
simi and
xuan-cao-swi
as code owners
| appraise 'dalli-latest' do | ||
| gem 'dalli' | ||
| end | ||
| # Dalli 4.2.0+ has native OpenTelemetry support, so we no longer need |
Contributor
There was a problem hiding this comment.
That's great!
I will have to look at how it gets auto loaded so we can incorporate as a solution for other gems
Contributor
There was a problem hiding this comment.
Should we update the package readme to inform users that if using dalli 4, we recommend to use the latest version as it contains native integration and development of this gem is frozen.
We could even restrict this gem to <= 4 so it is clear to use this. I note there was such a short period between v4 releases https://rubygems.org/gems/dalli/versions
| end | ||
|
|
||
| def add_patches | ||
| if dalli_has_native_otel_support? |
Contributor
There was a problem hiding this comment.
should this be in the compatible? block instead and add a MAX_VERSION constraint?
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Dalli 4.2.0 added Otel instrumentation 🎉
This PR disables our Dalli 4.2.0+ community instrumentation in favor of their native instrumentation to prevent double reporting.
I've opened an issue with the Dalli team to add their instrumentation to the Otel Registry, as well as noted differences in between the two instrumentations.