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

Support units in Instrument selector in Metric View #3101

Closed
asafm opened this issue Jan 13, 2023 · 3 comments · Fixed by #3184
Closed

Support units in Instrument selector in Metric View #3101

asafm opened this issue Jan 13, 2023 · 3 comments · Fixed by #3184
Labels
area:sdk Related to the SDK spec:metrics Related to the specification/metrics directory

Comments

@asafm
Copy link
Contributor

asafm commented Jan 13, 2023

What are you trying to achieve?
I would like to have the ability to say for a view, in its instrument selector: "Select all histogram type, which has the units 'seconds'".
This will allow me to say: All histograms with milliseconds unit with name matching "pulsar.messaging.*" should have the following buckets - 1, 5, 10, 20, 100, 1000, 5000.
Another example: All histograms with bytes unit should have the following buckets: 100 bytes, 500 bytes, 1kb, 2kb, and 10kb.

What did you expect to see?
In the section describing the instrumentation selector for a view, starting with "The Instrument selection criteria (required), which covers:" another bullet show be added saying "The unit of the instrument (optional)".

Additional context.

In my opinion, the ability to specify a bucket list based on units (seconds, bytes, milliseconds) combined with instrumentation scope or wildcard on a metric name is a very powerful and useful feature to have. Without it, and without hints API, one finds themself creating so many views, one for each histogram defined or trying to devise a naming scheme that would be matched using the name wildcard.

@asafm asafm added the spec:metrics Related to the specification/metrics directory label Jan 13, 2023
@asafm
Copy link
Contributor Author

asafm commented Jan 25, 2023

@SergeyKanzhelev What would be the correct way to move forward with this issue?

@asafm
Copy link
Contributor Author

asafm commented Jan 29, 2023

Small note. This is already supported without having spec for it in the Go SDK: https://github.com/open-telemetry/opentelemetry-go/blob/ec13377b6b7cea9ba5e6a2233ad934a411459a58/sdk/metric/view.go#L79-L85:

		matchFunc = func(i Instrument) bool {
			return re.MatchString(i.Name) &&
				criteria.matchesDescription(i) &&
				criteria.matchesKind(i) &&
				criteria.matchesUnit(i) &&
				criteria.matchesScope(i)
		}

@MrAlias
Copy link
Contributor

MrAlias commented Jan 31, 2023

Small note. This is already supported without having spec for it in the Go SDK

The Go SIG understood the following specification statement as allowing this matching:

OpenTelemetry SDK authors MAY choose to support more criteria. For example, a strong typed language MAY support point type (e.g. allow the users to select Instruments based on whether the underlying type is integer or double).

It might be worth explicitly adding unit to the matching if the goal is to have it in every language.

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

Successfully merging a pull request may close this issue.

4 participants