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

Add "get[Low|High]CardinalityKeyValue()" on "Observation.Context" #3505

Merged
merged 1 commit into from Oct 28, 2022

Conversation

ttddyy
Copy link
Contributor

@ttddyy ttddyy commented Oct 26, 2022

Add two new methods getLowCardinalityKeyValue and getHighCardinalityKeyValue.
Also, changes the internal holder from Set to Map.

Prior to this change, the usage of LinkedHashSet allowed multiple KeyValue with the same key. It was deduped when converted to KeyValues`, but it is not an ideal implementation.

This change replaces the LinkedHashSet with LinkedHashMap using the key from KeyValue as a map key. This way, when a new KeyValue is added, it will override the old KeyValue with the same key.

@sonatype-lift
Copy link
Contributor

sonatype-lift bot commented Oct 26, 2022

⚠️ 10 God Classes were detected by Lift in this project. Visit the Lift web console for more details.

@jonatan-ivanov jonatan-ivanov added the enhancement A general enhancement label Oct 27, 2022
@jonatan-ivanov jonatan-ivanov added this to the 1.10.0 milestone Oct 27, 2022
Add two new methods `getLowCardinalityKeyValue` and
`getHighCardinalityKeyValue`.
Also, changes the internal holder from `Set` to `Map`.

Prior to this change, the usage of `LinkedHashSet` allowed multiple
`KeyValue`s with the same key. It was deduped when converted to
`KeyValues`, but it is not an ideal implementation.

This change replaces the `LinkedHashSet` with `LinkedHashMap` using the
key from `KeyValue` as a map key. This way, when a new `KeyValue` is
added, it will override the old `KeyValue` with the same key.
@ttddyy ttddyy merged commit fd49b6f into micrometer-metrics:main Oct 28, 2022
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.

None yet

3 participants