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

need a way to avoid metric name collisions for /metrics/application when multiple apps exist #65

Closed
donbourne opened this issue Aug 29, 2017 · 11 comments
Assignees

Comments

@donbourne
Copy link
Member

Per discussion in MP metrics hangout call on Aug. 29:

In the spec, indicate that implementations that provide the ability to run more than one application can provide configuration for their server to enable an application GUID to be prepended to the application-scoped metric names

@mikecroft
Copy link
Member

Until now I'd just assumed that these paths were intended to be (effectively) /metrics/${app-name} and /metrics/${vendor_name} without really thinking of the implications of that kind of thing!

We could maybe consider a similar path to this kind of thing like /metric/application/${context-root} given that the context root must be unique in any server and is a bit more friendly than a GUID. WDYT?

@donbourne
Copy link
Member Author

Prepending the context-root is better than prepending a GUID. In the call we were discussing that, most of the time, there will just be one app per jvm -- which is why we thought making this a configurable option could make sense.

Given it's a context-root rather than a GUID, it might actually be tolerable to always prepend that -- to avoid people having to remember to reconfigure something when they add a second app to the server that might use some of the same libraries. Let's discuss on gitter...

@donbourne
Copy link
Member Author

Based on discussion in gitter, we'll aim to prepend the context root for all application-scoped metrics.

@donbourne
Copy link
Member Author

as an example, if the URL was http://localhost:8080/someApp/somepath?... the metrics for that app would be preceded by someApp.in the metric name. In the case where the context root is just / we don't prepend anything.

mikecroft added a commit to mikecroft/microprofile-metrics that referenced this issue Sep 7, 2017
mikecroft added a commit to mikecroft/microprofile-metrics that referenced this issue Sep 7, 2017
fixes eclipse#65

Signed-off-by: Mike Croft <backtotheoldhouse@gmail.com>
@mikecroft mikecroft self-assigned this Sep 7, 2017
@pilhuhn pilhuhn added this to the 1.1 milestone Sep 12, 2017
@pilhuhn pilhuhn modified the milestones: 1.1, 1.2 Nov 21, 2017
@pilhuhn
Copy link
Contributor

pilhuhn commented Jan 9, 2018

Also related: #169 and #29

@donbourne donbourne removed this from the 1.2 milestone Feb 2, 2018
@OndroMih
Copy link

@donbourne Why is this removed from 1.2 milestone? Since 1.2 isn't scheduled for MP 1.4 there should be enough time to find a solution for this and get it to 1.2.

Actually the PR #108 is already raised and the comments on it from @pilhuhn and me provide a non-breaking and simple solution.

@OndroMih
Copy link

I'm pushing for this as FT spec is hitting the same issue when multiple apps are deployed, as it defines metrics based on the application code and hence scoped to an application: https://github.com/eclipse/microprofile-fault-tolerance/pull/257/files

@jmesnil
Copy link
Contributor

jmesnil commented Mar 28, 2019

Just a note that I'd prefer we do not modify the metric name in that case.
We can add labels to distinguish between applications.
This allow a correct aggregation of metrics that is not possible if the metric name is different depending on its application.

Typical example is the http-count for each server deployed in my application (or applications).
I want to be able to aggregate the HTTP metrics either for a single servlet, an application or all my servlets doing either:

  • http-count (all servlets)
  • http-count[deployment=a.war] (all servlets in the a.war application)
  • http-count[deployment=a.war, servlet=foo.MyServlet] (only the foo.MyServlet in the a.war application)

It should be up to the MP Metric application to add the correct tags/labels to distinguish the metrics but this does not require to change the actual name of the metric.

(This also goes for the base, vendor and application scope that could go in a scope label instead of being prepended to the metric names...)

@donbourne
Copy link
Member Author

@jmesnil I'm very much in agreement with what you're saying about not requiring metric names to change to differentiate instances of a metric in different apps (or different servlets, per your example). I would have just given a +1 to the thumbs up, but don't want to confuse everyone as there's a parallel discussion going on about what the best place is to set the application label (app responsibility, server responsibility, or server-impl dependent?).

@jmartisk
Copy link
Contributor

jmartisk commented Mar 2, 2020

I think this is partially resolved by #362 - there is a recommendation to append a tag containing the deployment name to metrics owned by that deployment. This doesn't completely solve name collisions, because when metrics are distinguished by a tag only, they need to have the same metadata, so clashing metrics from different deployments would have to agree on some common metadata.
Is there anything more that we want to do here or can we close it?

@donbourne
Copy link
Member Author

I think #362 covers it. anyone can feel free to reopen with comments if they disagree.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants