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

Metrics API namespacing question #391

Closed
jmacd opened this issue Dec 16, 2019 · 7 comments
Closed

Metrics API namespacing question #391

jmacd opened this issue Dec 16, 2019 · 7 comments
Labels
spec:metrics Related to the specification/metrics directory

Comments

@jmacd
Copy link
Contributor

jmacd commented Dec 16, 2019

In the review for the Metrics SDK specification (#347), it became clear that "Named" Meters are not intended to serve as a namespacing function.

(cc: @danielkhan)

There is a requirement that is not currently well written in the Metrics API specification that metric names have a unique kind within a process. This can be done using the name of the named meter, but that was not the intention behind named meters. See the thread with @arminru:

#347 (comment)
#347 (comment)
#347 (comment)
#347 (comment)
#347 (comment)

One of the reasons not to use the named Meter's name, IMO, is that it implies different names for the "same" span when generated by different tracers. This has me wondering if a first-class namespace concept should be added to the API. This issue is intended as a placeholder for this topic without making a strong recommendation. I'm not sure how this should be handled.

@jmacd jmacd added the spec:metrics Related to the specification/metrics directory label Dec 16, 2019
@jmacd
Copy link
Contributor Author

jmacd commented Dec 16, 2019

@bogdandrutu ^^^ The suggestion to use the named Meter's name as a namespace for metric instruments came from you. There's now a debate over that topic.

@bogdandrutu
Copy link
Member

I think having namespace and a named tracer/meter will be very confusing. I am fine dropping entirely the named tracer/meter in favor of a better defined namespace if that is the proposal.

@danielkhan
Copy link
Contributor

danielkhan commented Dec 17, 2019

@bogdandrutu
Named tracers and Namespaces solve completely different use-cases.

Named tracers are a way to know which library reported a span or metric and (and this is not sufficiently specified yet) disabling or rate-limiting this tracer by configuration.

This implies that there can be more than one tracer and this means that there needs to be a registry / factory that keeps references to all tracers that exist.

Namespaces are used to create unique metrics and traces (as far as I understand) and this is a different problem to solve.

As such, you can not drop the one thing in favor of the other thing.

I agree that there is a lack of reasoning why named tracers were added to the spec in the first place and I would suggest that this should be added.

@dyladan
Copy link
Member

dyladan commented Jan 15, 2020

MOVED FROM #410

From the call today and other issues, there seems to be a lot of confusion around meter names vs namespaces. Some solutions have been proposed to deal with this but there seems not to be consensus which solves everybody's problems. I am opening this discussion issue in the hopes that we can resolve it quickly and move on with actual spec updates.

Here are the solutions that have been proposed that I am aware of:

Use meter name as namespace

I think this will probably be the way forward, and it seems to be the option most people are in favor of.

This is appealing because it completely removes the concept of namespace as a separate thing. It is on library developers to make sure that metric names within their library are unique, and if two libraries choose the same name, like "latency", then they are unique by virtue of being registered by separate libraries.

Examples:

  • Library mongodb registers latency. Produced metrics are namespace=mongodb, name=latency
  • Library postgresql registers latency. Produced metrics are namespace=postgresql, name=latency

Problems:

  • If two libraries (mongodb-core and mongodb-native) both export latency and you want them to show up as a single latency metric. Perhaps this can be solved on the metrics backend or using labels?
  • You are migrating from one library to another and you want the metrics provided by the new library to show up as the same metric as the old library. Perhaps this can be solved in the metrics backend or using labels as above?

Require metric names to be fully qualified

In this scenario, it is entirely on library developers to give their metrics a name which is globally unique. This simplifies implementation, but could be problematic if a library developer decides to use a name that is too general or misunderstands the way the system works.

Examples:

  • Library mongodb registers mongodb_latency
  • Library postgresql registers postgresql.latency

Problems:

  • Two libraries try to register the same name (mongo and psql both try to register db_latency)

Use meter name as namespace, but allow it to be overridden by the operator

In this scenario, it is again up to library developers to make their metric names unique only within their library, but allows operators to override the namespace used by a given library for one reason or another.

Examples:

  • Library mongodb registers latency. Produced metrics are namespace=mongodb, name=latency
  • Library mongodb-core and mongodb-native both register latency. Produced metrics are namespace=mongodb-core, name=latency and namespace=mongodb-native, name=latency respectively.
    • The user overrides the namespace of both libraries to be mongodb. Produced metrics are now namespace=mongodb, name=latency for both of these libraries.

Problems:

  • If two libraries each export a counter named count, and the user configures them to have the same namespace, does the meter need to return the same counter for both of them? If one of them increments 5 times and the other increments 3, is the value of the count counter 3, 5, or 8?

Example mock configuration:

// operator initializes OT SDK
opentelemetry.initGlobalMeterProvider(
  new MeterProvider({
    namespaceOverrides: [
      {
        meterName: 'mongodb-native',
        namespace: 'mongodb'
      },
      {
        meterName: 'mongodb-core',
        namespace: 'mongodb'
      }
    ],
  })
);

// In library named mongodb-core
const coreMeter = opentelemetry.getMeter("mongodb-core", "3.2.2");
const coreLatency = coreMeter.createMeasure("latency");

// In library named mongodb-native
const nativeMeter = opentelemetry.getMeter("mongodb-native", "1.2.0");
const nativeLatency = nativeMeter.createMeasure("latency");

// In library named postgresql
const pgMeter = opentelemetry.getMeter("postgresql", "5.2.4");
const pgLatency = pgMeter.createMeasure("latency");

// The `namespace` property may not actually be exported, but is
// included here for illustrative purposes.
console.log(coreMeter.namespace); // mongodb
console.log(nativeMeter.namespace); // mongodb
console.log(pgMeter.namespace); // postgresql

/**
 * In this example, the two mongodb libraries have been configured
 * to have the same namespace by the operator. If the operator
 * had not configured namespace overrides, they would have had their own
 * separate namespaces.
 */

@dyladan
Copy link
Member

dyladan commented Jan 15, 2020

Based on what we talked about in the call, I believe we are moving forward with the "Use meter name as namespace" option. I am drafting a small update to the spec that makes this more clear and removes the ambiguous component.

@Oberon00
Copy link
Member

One thing (I think the only one) that worries me when merging namespace and meter name is that for the namespace I would prefer the name of the instrumented library instead of the instrumenting library. But I guess one can live with that too.

@dyladan
Copy link
Member

dyladan commented Jan 28, 2020

I think this is closed by #408

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
spec:metrics Related to the specification/metrics directory
Projects
None yet
Development

No branches or pull requests

5 participants