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

Provide an option to supply common tags for registries. #3744

Closed
lenin-jaganathan opened this issue Apr 6, 2023 · 11 comments
Closed

Provide an option to supply common tags for registries. #3744

lenin-jaganathan opened this issue Apr 6, 2023 · 11 comments
Labels
superseded An issue that has been superseded by another

Comments

@lenin-jaganathan
Copy link
Contributor

Please describe the feature request.
This thought arrived during a discussion on #2818. The thought process is that MeterRegistries can take common tags static to a JVM (like app_name, host_name, region, manifest, and whatnot) and use that to append as metadata while sending metrics (if the back-end supports that) or at least use those common tags to get translated to corresponding systems tag equivalent once and use it for every publish.

Rationale

  • Some backends have provisions to add common metadata once per request, so we can avoid referencing tags/creating new ones if we have common tags.
  • Some registries convert the tags to their system equivalent thing (dimensions/labels) etc and they do it for every meter. This generates too much garbage as this is only needed to send the data for that Step. If static objects were created during the initialization, the same objects can be attached to all the metrics and we can avoid allocations.
  • Sometimes, users ended up creating multiple tag objects with the same tag key and name this extra allocation to go to Old gen, these can be completely eliminated
@lenin-jaganathan
Copy link
Contributor Author

Well, this issue is subtly different from the existing "commonTags" method which adds the commonTags to all the meters which are processed once every step interval.

@marcingrzejszczak
Copy link
Contributor

Why won't a MeterFilter be a good idea to add such tags?

@marcingrzejszczak marcingrzejszczak added the waiting for feedback We need additional information before we can continue label Dec 20, 2023
@lenin-jaganathan
Copy link
Contributor Author

Because it is not so efficient to do it every time we interact with the meter. A 1x1 example is OpenTelemetry's Resource, where unique information can be store once and re-used multiple times.

If we try to do this in a meterfilter, every single time a meter needs to be recorded we have to run through the filter and add tags and them record the value on it.

E.g: Let's I have a service fooserv and I need this info to be added to all the meters.

If I add a meter filter, every time I want to record metric for a HTTP request, initially i would create a meter with information like path, status and etc. When I call register on the meter, the filters get executed and it tries to add a new tag service=fooserv. Since Meter.Id is immutable this creates a new object and registers it. This is the additional hop I am talking about.

Alternatively, if We can say to the registry that service=fooserv is needed on all the metric, it can add that information during export time alone to the actual protocol it talks to.

Copy link

github-actions bot commented Jan 2, 2024

If you would like us to look at this issue, please provide the requested information. If the information is not provided within the next 7 days this issue will be closed.

@lenin-jaganathan
Copy link
Contributor Author

@marcingrzejszczak How can we prevent an auto close on this issue?

Copy link

If you would like us to look at this issue, please provide the requested information. If the information is not provided within the next 7 days this issue will be closed.

Copy link

Closing due to lack of requested feedback. If you would like us to look at this issue, please provide the requested information and we will re-open.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Jan 24, 2024
@izeye
Copy link
Contributor

izeye commented Jan 25, 2024

This seems to have been closed unintentionally by the bot.

@jonatan-ivanov jonatan-ivanov removed waiting for feedback We need additional information before we can continue feedback-reminder closed-as-inactive labels Jan 25, 2024
@jonatan-ivanov
Copy link
Member

We need to look into why the bot ignores comments, it seems it closes every issue that has the waiting for feedback label. :(

@jonatan-ivanov
Copy link
Member

@lenin-jaganathan Do you think this issue is similar/related to these two?

@lenin-jaganathan
Copy link
Contributor Author

Yeah more like #5112. I will close this.

@lenin-jaganathan lenin-jaganathan closed this as not planned Won't fix, can't repro, duplicate, stale Jul 2, 2024
@jonatan-ivanov jonatan-ivanov added superseded An issue that has been superseded by another and removed feedback-provided labels Jul 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
superseded An issue that has been superseded by another
Projects
None yet
Development

No branches or pull requests

4 participants