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

Consider allowing a simpler API for histogram default buckets #3621

Closed
dashpole opened this issue Jul 24, 2023 · 6 comments · Fixed by #3662
Closed

Consider allowing a simpler API for histogram default buckets #3621

dashpole opened this issue Jul 24, 2023 · 6 comments · Fixed by #3662
Assignees
Labels
spec:metrics Related to the specification/metrics directory

Comments

@dashpole
Copy link
Contributor

Context

This is feedback from a Go prototype of the Instrument Advice API:

Feedback

When prototyping in Go, "Advice" itself seemed unnecessary for allowing histogram bucket boundaries to be configured on histogram instruments. The simplest, cleanest API was to make bucket boundary configuration a "top-level" configuration option exclusive to histograms, rather than nested within an "Advice" concept.

The ask in this issue is to at least allow API implementations to exclude the notion of Advice, and instead allow directly configuring the subfields of Advice on the instruments they apply to.

cc @jack-berg @MrAlias @MadVikingGod

@MrAlias
Copy link
Contributor

MrAlias commented Jul 25, 2023

"When prototyping in Go, 'Advice' itself seemed unnecessary for allowing histogram bucket boundaries to be configured on histogram instruments" should be "When prototyping in Go, 'Advice' itself seemed unnecessary for allowing histogram bucket boundaries to be configured on anything but histogram instruments", right?

@dashpole
Copy link
Contributor Author

I meant to say that configuring buckets directly, rather than nested inside of advice, was simpler. I.e. advice was not necessary for allowing buckets to be configured on histograms.

Your statement is also correct that histogram bucket boundary configuration didn't make sense to include as options for non-histogram instruments.

@MrAlias
Copy link
Contributor

MrAlias commented Jul 25, 2023

I meant to say that configuring buckets directly, rather than nested inside of advice, was simpler. I.e. advice was not necessary for allowing buckets to be configured on histograms.

Your statement is also correct that histogram bucket boundary configuration didn't make sense to include as options for non-histogram instruments.

Gotcha 👍

@trask
Copy link
Member

trask commented Aug 13, 2023

just fyi, I've added this to the spec meeting agenda for discussion since it's potentially a blocker for #3391 which is a blocker for HTTP semconv stability

@jmacd
Copy link
Contributor

jmacd commented Aug 15, 2023

The ask in this issue is to at least allow API implementations to exclude the notion of Advice, and instead allow directly configuring the subfields of Advice on the instruments they apply to.

This seems reasonable.

@dashpole
Copy link
Contributor Author

Notes from the SIG meeting:

  • We are OK loosening language around how the parameter is passed.
  • We need to ensure that it is still clear to users that the parameter can be ignored by SDKs.

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

Successfully merging a pull request may close this issue.

5 participants