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

Property based method invocation counter #8687

Open
breedx-splk opened this issue Jun 9, 2023 · 5 comments
Open

Property based method invocation counter #8687

breedx-splk opened this issue Jun 9, 2023 · 5 comments
Labels
contribution welcome Request makes sense, maintainers probably won't have time, contribution would be welcome enhancement New feature or request

Comments

@breedx-splk
Copy link
Contributor

This feature would introduce a new configuration property that allows users to measure the number of invocations of their methods without having to modify source code. This will be similar to otel.instrumentation.methods.include, but just a light weight metric and not a full-fledged span.

Propose config item: otel.instrumentation.method.counters. The format of the value should be identical to the format of otel.instrumentation.methods.include (eg. my.package.MyClass1[method1,method2];my.package.MyClass2[method3]).

Propose metric name: my.pakage.ClassName.method.invocations type = counter (monotonically increasing).

@breedx-splk breedx-splk added enhancement New feature or request contribution welcome Request makes sense, maintainers probably won't have time, contribution would be welcome labels Jun 9, 2023
@trask
Copy link
Member

trask commented Jun 9, 2023

my wish list would include changing the methods instrumentation to read its configuration from a yaml file (or wait until we have standard yaml file). e.g. I've also had interest in being able to specify the SpanKind for otel.instrumentation.methods.include but don't want to squash that into the existing single property

@xiepuhuan
Copy link
Contributor

xiepuhuan commented Jan 21, 2025

I want to solve this problem, but I think we should use DoubleHistogram to measure the duration and number of invocations.
@trask

@trask
Copy link
Member

trask commented Jan 21, 2025

@xiepuhuan that sounds great!

@xiepuhuan
Copy link
Contributor

xiepuhuan commented Jan 22, 2025

#11354

In this PR, I see that @Counted and @Timed are used to measure the number and duration of invocations of method, and the attribute of the metric contains code.namespace,code.function,exception.type, I think the metric created by configuration should be consistent with them.The following three points need to be achieved.

  1. Use a fixed short and meaningful metric name. Example: code.method.invocation.duration
  2. Metric attributes include code.namespace,code.function,exception.type
  3. Use otel.instrumentation.method.counters and otel.instrumentation.method.timers to configure the number and duration of invocations of method.

When using a query similar to promql, the class name and method name can be used as attributes for filtering and aggregation operations.
@trask I would like to know your opinions.

@trask
Copy link
Member

trask commented Jan 24, 2025

  1. Use a fixed short and meaningful metric name. Example: code.method.invocation.duration

I think we went away from this proposal in #11354, probably because:

"As a rule of thumb, aggregations over all the attributes of a given metric SHOULD be meaningful," as Prometheus recommends.

I think it's better to just make metric name one of the configuration options

  1. Metric attributes include code.namespace,code.function,exception.type

let's use error.type instead of exception.type (we should do the same in #11354), this is what we're putting on metrics in semantic conventions

  1. Use otel.instrumentation.method.counters and otel.instrumentation.method.timers to configure the number and duration of invocations of method.

I didn't follow this part?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contribution welcome Request makes sense, maintainers probably won't have time, contribution would be welcome enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants