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

Question: naming conventions, prefer . (dot) or _ (underscore)? #2740

Closed
jviau opened this issue Aug 17, 2022 · 8 comments
Closed

Question: naming conventions, prefer . (dot) or _ (underscore)? #2740

jviau opened this issue Aug 17, 2022 · 8 comments
Assignees
Labels
area:semantic-conventions Related to semantic conventions

Comments

@jviau
Copy link

jviau commented Aug 17, 2022

I have a question regarding naming conventions. When looking at FaaS metrics sematic conventions here, I see the names uses a mix of . and _. So my question is:

Why was a name like faas.invoke_duration chosen over faas.invoke.duration?

As I add my own metrics, I want to make sure I understand this rationale and know when to use . vs _ for names.

@jviau jviau added the spec:miscellaneous For issues that don't match any other spec label label Aug 17, 2022
@jviau
Copy link
Author

jviau commented Aug 17, 2022

I also just noticed it is not consistent: RPC spec follows rpc.server.duration here.

So I am guessing this is just in progress?

@reyang
Copy link
Member

reyang commented Aug 17, 2022

The minimum bar is captured by the API spec https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/api.md#instrument-naming-rule.

My general observation is that folks intend to use . to group things in hierarchical way, and _ is used to improve readability if a particular term has multiple words.

Some additional things to consider while exporting the data to specific backends, e.g. https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/data-model.md#otlp-metric-points-to-prometheus

@reyang
Copy link
Member

reyang commented Aug 17, 2022

I also just noticed it is not consistent: RPC spec follows rpc.server.duration here.

So I am guessing this is just in progress?

Yes, if you scroll to the very top of this spec, you can see it marked as Experimental instead of Stable, which means it could have breaking changes or even get removed or redesigned in the near future.

@Oberon00
Copy link
Member

Probably it makes sense to apply the same logic as with attribute naming: https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/common/attribute-naming.md

@jviau
Copy link
Author

jviau commented Aug 19, 2022

Thank you for the links and explanation.

@reyang - regarding rpc.server.duration vs faas.invoke_duration. Both are marked as "Experimental". Would you know, or be able to guess, which direction OTel will settle on? (I personally am more of a fan of rpc.server.duration).

@joaopgrassi
Copy link
Member

joaopgrassi commented Aug 22, 2022

My interpretation is also as @reyang mentioned. A dot . is for grouping or "namespacing" things together. Your example of rpc.server., it has a dot after server, because server is the grouping of multiple metrics like duration and requests_per_rpc.

For the faas.invoke_duration I'd imagine that it was chosen like that because invoke alone is not a "grouping". If there was more sub-metrics under invoke then it would make sense to use what you propose, faas.invoke.duration. But since it's just a concept on itself then it's under the top-level faas. An underscore then is how one can combine multiple words.

@joaopgrassi joaopgrassi added area:semantic-conventions Related to semantic conventions and removed spec:miscellaneous For issues that don't match any other spec label labels Aug 22, 2022
@jviau
Copy link
Author

jviau commented Aug 22, 2022

faas invoked does have a grouping concept, at least in the attributes: https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/semantic_conventions/faas-metrics.md#attributes faas.invoked_name, faas.invoked_provider, and faas.invoked_region. I could also see more metrics based on invoked, just like rpc.server.* - could have faas.invoked.request.size and faas.invoked.response.size

But this issue isn't to argue/discuss the choice of FaaS naming. I have my answer: . when it is a grouping, _ when it is multi-word separation. What is a grouping vs multi-word is always going to be subjective. Thank you for the answers.

@jviau jviau closed this as completed Aug 22, 2022
@Oberon00
Copy link
Member

For FaaS, "invoked" is not really suitable as a namespace IMHO. "faas.invoked.request.size" = the size of the invoked request? That does not make sense. Maybe it would be better to rename this to faas.invocation.request.size, faas.invocation.peer.provider, etc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:semantic-conventions Related to semantic conventions
Projects
None yet
Development

No branches or pull requests

5 participants