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

Enable custom registries #574

Open
rmannibucau opened this issue Apr 16, 2020 · 22 comments
Open

Enable custom registries #574

rmannibucau opened this issue Apr 16, 2020 · 22 comments

Comments

@rmannibucau
Copy link
Contributor

Hi,

Today registries are bounded to an enum. This can be bothersome since it just moves the "namespace" issue to another namespace (I'm thinking to vendor but I'm sure application hits that too).

My proposal is to keep the enum because for very small services it is common but also enable to name it with a plain string.
User would register its registry on any CDI bean and the metrics impl would capture this and register an instance with this name. It would also add it to exporters using its name as prefix (similarly to the enum usage).

here is an example:

@CustomMetricRegistryRegistration("myapp") // on any *CDI bean*
 public class Register {}

Then the usage is to add everywhere we have Type the corresponding String (means we also have type = CUSTOM):

@Inject
@RegistryType(type=MetricRegistry.Type.CUSTOM, custom = "myapp")
private MetricRegistry registry;

I suspect that @RegistryType qualifier can also become a parameter of the interceptor bindings (counted, metered etc) but this can be another issue.

@jmartisk
Copy link
Contributor

Lately we've heard some voices suggesting basically the opposite - dropping the notion of having multiple registries. Micrometer does not have them either, for example (at least not in the meaning of namespaces for metrics).
I'd agree the current state of three static registries is somewhat inflexible, we might have to choose whether to reduce to just one registry, or to introduce these custom registries.

Given how MicroProfile is now shifting toward an implementation-first approach, this sounds like a good potential experiment to include in one implementation and then consider backporting it to the spec if it proves useful. WDYT?

@rmannibucau
Copy link
Contributor Author

Hmm, we have only two options: either enable custom registries and keep the consistency to be able to categorize metrics or flatten them all and use the naming as category. Both works well. Now I think we should consider the user impact. Today, early adopters already did dashboards and if we break it to use a single registry then it will be to redo (I'll emphasis this point being more concrete: this would mean dropping microprofile in several companies due to the regular breaking changes making it impossible to use without regular investment). Just for that i would already keep what we have and just extend the current API to enable custom metrics.

Now functionally, MP is about infra so must be usable by all layers of a stack and integrable by 3rd parties. This means being able to seggregate data makes sense and since MP is not really about the exporter (it only supports prometheus but I saw some users exporting to graphite and friends), it is a good thing to keep the seggregation abstracted and let the exporter interprete it as needed (flattening a key or using the registry as a layer in JSON/XML/Avro...

However, at geronimo we got an issue which is linked to this topic but is more generic: how to remap a MP metric name to something else (company choice for memory, cpu, datasource pool etc). In geronimo we enabled to deploy a mapping file (mpname => exporter name). I think this is more important than using a single registry.

So personally i'm for:

  1. support custom registry
  2. enable interceptor to use any registry
  3. add a mpconfig integration to support mapping of metrics name in openmetrics exporter (abstracted or not, i'm not yet sure, could be interface MetricsMapper { Optional<String> map(String) } and handled by the impl as a chain grabbing all deployed impl in CDI context)

hope it makes sense

@donbourne
Copy link
Member

@rmannibucau , why do we need custom registries? Just want to make sure we have a clear demonstrated use case.

@rmannibucau
Copy link
Contributor Author

@donbourne ok, so you have 2 kinds of guys: the ones who want to have a single registry and the ones who want to have subcategories (to split it per team/concern/... as explained before). In the category of people who want N registries, they rarely want base (which does not mean anything ;)), vendor (which is worse ;)) and application. Today base is more or less the JVM, vendor the microprofile vendor, application the final business code so where do you put a stack, a library, a team integrating with another team? Indeed you can put them in application or vendor but it is wrong and reduces the lisibility of metrics. This is why it is important to enable custom registries.

@donbourne
Copy link
Member

@donbourne ok, so you have 2 kinds of guys: the ones who want to have a single registry and the ones who want to have subcategories (to split it per team/concern/... as explained before).

Ultimately they need to export the metrics, for example to /metrics endpoint. Do you see teams wanting to make queries on just their namespace of metrics? I'm certainly aware of the common case of just hooking up Prometheus to call /metrics... but I'm less sure about how many ops teams would want to query on a subset of the metrics available from a server. Especially as servers are more typically serving a microservice -- where the rationale for custom registries seems less evident to me (since if a server is only doing one thing I likely just want it to tell me all of its metrics).

@rmannibucau
Copy link
Contributor Author

@donbourne well MP-metrics exposes both endpoints (more actually) so not a big deal but to be honest the only case I know is to query them all but be able to quickly filter the metrics by name in grafana or other reporters (thinking to graphite out of my head too). Having a category ending as a prefix works well. If you merge everything without the registry as prefix you quickly have conflicts and usage is less smooth.

@donbourne
Copy link
Member

In grafana you decide what to put into the query for each visualization (generally based on a single metric or perhaps one metric divided by another metric). So I'm not worried about filtering there.

Having a category ending as a prefix works well.

I'm not sure what you mean exactly -- example?

@rmannibucau
Copy link
Contributor Author

When building grafana you often use the completion, not having categories is the open door to a mess (categories really help ops here).

Just meant that dropping categories notion works technically but makes human interactions (dashboards, discovery, ...) hurtful compared to using categories from my past experience.

@donbourne
Copy link
Member

@rmannibucau , I think we generally agree. So back to your point...

Today base is more or less the JVM, vendor the microprofile vendor, application the final business code so where do you put a stack, a library, a team integrating with another team?

I would say the best scenario would be that EVERY metric just start with a component prefix. Eliminate base, vendor, app as prefixes. If you have a library or stack that wants to define metrics related to a component, start the metric name with a prefix corresponding to that component.

Would you agree with that?

@rmannibucau
Copy link
Contributor Author

@donbourne this is where we come from and got a real benefit to not have this rule but enforce it with isolated registries cause otherwise it is too hard to ensure all submetrics (some not coming from the app) follow that arbitrary rule. On the opposite, isolating per registry guarantees it even if not metrics naming+registration was not designed to follow that pattern.

@donbourne
Copy link
Member

so with custom registries, would the name of the custom registry be used as the metric prefix? It feels a bit like that's imposing an artificial grouping construct (the registry) on the ops person trying to figure out what they want in their dashboard.

@rmannibucau
Copy link
Contributor Author

@donbourne I agree with that...but it is true for single repository deployments too as explained. This is why I mentionned a transversal need regarding the naming is a mapper (name -> exporter name) as geronimo-metrics has by default by config because whatever we do, when composing metrics names will never be exactly the expected ones so a mapper is needed, but it does not remove the fact that a default with enforces cat is a good thing.

@donbourne
Copy link
Member

@rmannibucau , thinking about this more -- what about using tags to identify what you're proposing we use custom repos for? The benefit of using tags is:

  • compatible with past MP metrics releases - doesn't break dashboards
  • people can choose, in Grafana, to either use or ignore the tag
  • avoids need for new way to get to new registries in /metrics (which existing tools may not be compatible with)

@rmannibucau
Copy link
Contributor Author

@donbourne I'm not sure tags are reliable. Basically all custom exporters I saw were ignoring them + tags can just be a child so you still conflict at key name + it does not enable to isolate the metrics between segments of the applications in terms of API so sounds like a short term workaround but not a long term solution to me.

@donbourne
Copy link
Member

@rmannibucau , to clarify which custom exporters do you mean? One could write a custom exporter that, for example, only returns metrics with MetricIds containing a certain tag name/value. The tags are a first class part of the MetricId, so you can't have 2 different metrics with the same Name/Tag combo.

Similarly, I could use a MetricFilter to only return the metrics that have a particular tag name/value if my use case relates to using the API.

@rmannibucau
Copy link
Contributor Author

@donbourne was thinking to custom graphite and kafka ones precisely. Issue with tag is that depending how you use them, you want or not them to appear. Tag are generic enough to be used by the app, by exporters, to communicate between layers of the app etc. Using them right here requires way more efforts than what we can ask for metrics IMHO. Typically the common example in the TCK is the environment, which is pointless in most deployments since you have it elsewhere (basically when you define the polling you also define the env and likely customize colors for examples). Indeed we can design anything with tags but it is the same topic than using a single registry and splitting registries is exactly sane on an API point of view here for consumers and producers.

@donbourne
Copy link
Member

@rmannibucau , so is your use case that you want to have ability to define a custom registry "X" and then be able to view that using /metrics/X ?

@rmannibucau
Copy link
Contributor Author

@donbourne my use case is to be able to pass a registry instance and ensure it is isolated (= its data are a consistent set for consumers and producers/contributors). I don't use microprofile exporters (either cause I use another underlying technology or because I use a frontend which is not JAX-RS) so I don't care - personally - of /metrics. That said I expect the API to just be a qualifier so /metrics trivially will know all these registries and will be able to handle them properly. It is 1-1 with the current enum, just an extension of it. It is very close to this API which enables either an enum usage or a string based usage when the enum=custom: https://github.com/Talend/component-runtime/blob/master/component-api/src/main/java/org/talend/sdk/component/api/component/Icon.java#L36.

@ebullient
Copy link

@rmannibucau .. I would like to walk you through how micrometer works, to see if you can get the behavior you want with that as an abstraction pattern instead. Micrometer takes a different organization/configuration/filter approach, and I would love to see if it can fit your use case.

@rmannibucau
Copy link
Contributor Author

@ebullient if you have a link I would be happy to review it but last time I checked - was not this month though ;), micrometer was doing exactly the opposite of what I'm asking (ie enable to merge multiples repo in a (vitually) single one which I hope to avoid because I want to be able to have independent lifecycles, independent registrations, and support conflicting keys, ie enabling each metric contributor to provide the same keys and the unicity is handled thanks the registry, so the registry has an unique id but you don't need to keep a unique id prefix in all your registrations - I know it is equivalent but in terms of investment it is way more efficient to do it at registry level than metrics, 1 vs N in terms of complexity and storage).

@ebullient
Copy link

ebullient commented Jul 30, 2020

I'm not sure I follow exactly, so a conversation is probably more effective. A micrometer registry can definitely be used to manage adding prefixes to names. https://micrometer.io/docs/concepts#_transforming_metrics

@rmannibucau
Copy link
Contributor Author

@ebullient let's find a way to exchange more interactively through twitter messages then we'll report there the conclusions.

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

No branches or pull requests

4 participants