Clarify name/version identity for named tracer/meters.

[carlosalberto]

From OTEP 76, Bogdan's comments:

is the name the key that identifies the Tracer/Meter or name + version? What is the expected behavior when (I don't know how is it possible) there are 2 requests to the getTracer with same name and different versions?

and

I think it is important mainly for the metrics side. One important thing we need to ensure the name of the metrics are unique inside a "component" (instrumentation library), so if the component is identified by the name then I need to keep a list of all the registered instruments at the name level vs at the name+version level.

Please clarify this when including the related changes into the specification.

[dyladan]

Please assign this to me so I don't lose it

[andrewhsu]

from our spec sig mtg, seems like ongoing discussion in gitter before this can be closed

[jkwatson]

In relation to the name of Meter, and the corresponding instruments that are spawned from that meter:

It seems to me like there are two approaches to the naming schemes here, and the semantics of the names.

1. The name of the meter is required to be included in the semantics of the instrument names for the names of the resulting metrics to make sense. For example, you could name the meter "okhttp-http-client", and then the instrument names would look like "latency" and "request_count". In this case, exporters will need to append the meter name to the instrument name for a meaningful resulting metric name.
2. The name of the meter is purely used for namespacing, and it not required to be used to form meaningful metric names. For example, you name the meter "okhttp-client", and the instrument names are "http_request_latency" and "http_request_count". In this case, exporters may use the meter name as an attribute/label if desired, but the names of the metrics will not need to be changed for them to be meaningful.

Personally, I think 2) makes more sense, and makes it feel more like where we landed with tracer naming, but a contrib implementation in the java repo takes approach 2: https://github.com/open-telemetry/opentelemetry-java/blob/e248ea7ad74b3ac9774f871636c8f6b839be998f/contrib/runtime_metrics/src/main/java/io/opentelemetry/contrib/metrics/runtime/MemoryPools.java#L67

I think we should clarify this somewhere in the specification, since there isn't clarity right now.

[dyladan]

Where @bogdandrutu and I had landed on this is that the meter name is the name of the library, just like tracers, but is also used to namespace the instrument names.

Example library named fancy-db-instr:

const meter = api.metrics.getMeter('fancy-db-instr', 'semver:1.0.0');

const disk = meter.createObserver('index_size', {
  monotonic: false,
  labelKeys: ['index_name'],
  description: 'Size in gb of an index',
});

function getIndexSize(name: string) {
  // return index size
}

disk.setCallback((observerResult) => {
  observerResult.observe(() => getIndexSize('idx1'), { index_name: 'idx1' });
  observerResult.observe(() => getIndexSize('idx2'), { index_name: 'idx2' });
  observerResult.observe(() => getIndexSize('idx3'), { index_name: 'idx3' });
  observerResult.observe(() => getIndexSize('idx4'), { index_name: 'idx4' });
});

At export time, this metric would be namespaced using fancy-db-instr, which would make it unique from some other library which has used the metric name index_size.

[jkwatson]

This seems the opposite of where we landed for Tracer names, where the intention is not to have to prepend tracer names on to span names to make them meaningful. Are we comfortable with the fact that these two approaches are semantically different?

[dyladan]

I think the issue is that if metric names collide, it could cause major issues in backends like prometheus. Not so for span names. So we need to prevent metric name collisions, and it would be weird for users if we made them have a name and a namespace. Also, there would be no guarantee that different libraries wouldn't try to squat on the same namespace, so you just reintroduce the possibility of collisions again. If we use the library name, we know the namespace is unique.

[jkwatson]

can't we make it so this is something that prometheus exporters do, rather than baking it in and forcing all exporters to do it? I would think that most backends could have a label/attribute to distinguish the metrics.

[dyladan]

It doesn't necessarily need to be prefixed to the name. This is a concern of the exporter. However an exporter does namespacing, they should use the meter name as that namespace.

[jkwatson]

If the instrument is named "latency", then you'll have to prefix the meter name to make a meaningful metric name. And, you'll get a different http_client_latency metric name for every instrumentation library, across every language. That seems like a huge downside to this approach.

[jkwatson]

Let's rephrase: if a backend wants unified metric names across libraries & languages, it won't have a way to do it, with this strategy.

[dyladan]

Option 2 opens up the possibility of naming collisions. If mongodb and mysql both export metric names db_read_latency, the names will collide.

[jkwatson]

The New Relic backend wants those names to be the same, distinguished by labels. How can we accomplish that with this approach?

[jkwatson]

Option 2 opens up the possibility of naming collisions. If mongodb and mysql both export metric names db_read_latency, the names will collide.

So, the prometheus exporter can prepend if it needs to, due to a limitation on the backend.

[dyladan]

The newrelic exporter would simply not prepend the meter name if it has some other method of disambiguating collisions

[jkwatson]

but "latency" is a terrible metric name. We want "http_client_latency" to be a separate metric name from "http_server_latency". I don't see a way to make this happen with this approach, unless we strictly specify the structure of the meter names.

[dyladan]

We can't guarantee that users will give good and meaningful names. That is a separate issue. Or else I am completely misunderstanding your issue here and we're back at square 1

[jkwatson]

The example in the java repo has the instrument names as "area" and "pool", scoped by the meter name "jvm_memory". The instrument names are completely meaningless without prepending the meter name. I'd like to change the instrument names to "jvm.memory.area" and "jvm.memory.pool", but @bogdandrutu pushed back and said I should just prepend at export time.

[dyladan]

Here is how I would go about that example. maybe it will help clarify my thinking.

First, that example is not using the name of the instrumenting library as the meter name, which is a spec violation. So I would change to getMeter('io.opentelemetry.contrib.metrics.runtime', version). (sorry I don't know where to find the version to complete the example, but version is optional anyways per-spec.

Second, I would change the metric name on line 75 to be more meaningful. Possibly "jvm_memory_area" or similar. "area" is a meaningless name on its own.

[jkwatson]

It sounds like we are 100% in agreement then! How to we make sure that this approach is codified somewhere, so there isn't confusion on this?

[dyladan]

Is that not how you interpret this? https://github.com/open-telemetry/opentelemetry-specification/blob/bdf39cb935a6f48badc687daeb78a66dc352761f/specification/metrics/api-user.md#obtaining-a-meter

[jkwatson]

It's definitely how I interpreted it! But, @bogdandrutu didn't, I guess.

[jkwatson]

I think we could add some clarity to the metric instrument name section, since that's really where the disagreement lies, I think. Also, semantic conventions for instrument names are definitely required.

[tsloughter]

+1 to @jkwatson and @dyladan 's interpretation. It is also how I've been using it.

Important for us because Tracers and Meters are named and versioned based on the instrumenting libraries OTP Application name (like a Java package I think...) and then each module in the Application is mapped to the Tracer/Meter so when a tracer or meter operation is done in a module the code simply has to say, "lookup Tracer/Meter by the name of the module I'm in". But this does make using the meter name as a prefix annoying and better moved to a label.

[dyladan]

But this does make using the meter name as a prefix annoying and better moved to a label.

Note that the spec only says the meter name should be used to namespace metrics. It doesn't specify a mechanism for that. In my mind, it is the responsibility of the exporter to make sure it is using a mechanism appropriate to its target backend.