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

MetricName TagMap supports replacement of existing entries #1537

Merged
merged 5 commits into from
Aug 1, 2022

Conversation

carterkozak
Copy link
Contributor

Why?

I've been thinking about adding additional data to our metrics describing the jvm runtime.
Some potential approaches would introduce risk that existing tags are repeated, although I suspect we'd use an approach including validation within metric-schema. In the event that there's some risk of key overriding, we'd want tritium to support it gracefully long before anything else relied on the functionality.

Before this PR

SafeIllegalArgumentException: Base must not contain the extra key that is to be added

After this PR

==COMMIT_MSG==
MetricName TagMap supports replacement of existing entries
==COMMIT_MSG==

Possible downsides?

Slightly more code in withEntry could be harder for the jvm to optimize

@changelog-app
Copy link

changelog-app bot commented Aug 1, 2022

Generate changelog in changelog/@unreleased

Type
See change types. Select one:

  • Feature
  • Improvement
  • Fix
  • Break
  • Deprecation
  • Manual task
  • Migration

Description

MetricName TagMap supports replacement of existing entries

Check the box to generate changelog(s)

  • Generate changelog entry

Copy link
Contributor

@schlosna schlosna left a comment

Choose a reason for hiding this comment

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

Overall looks good. Have a couple doc & test nits

@carterkozak carterkozak marked this pull request as ready for review August 1, 2022 20:31
@bulldozer-bot bulldozer-bot bot merged commit 756333e into develop Aug 1, 2022
@bulldozer-bot bulldozer-bot bot deleted the ckozak/tagmap_replace_existing branch August 1, 2022 20:34
@svc-autorelease
Copy link
Collaborator

Released 0.50.0

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.

4 participants