open-telemetry / oteps

OpenTelemetry Enhancement Proposals
https://opentelemetry.io
Apache License 2.0
337 stars 164 forks source link

Proposal for additional deployment standard attr #203

Closed bodepd closed 1 year ago

bodepd commented 2 years ago

This commit create a design proposal for adding a new deployment attribute in order to be able to consider deployment.environments via 2 dimensions as opposed to one.

bodepd commented 2 years ago

I would be opposed to this change.

  1. In my last two jobs the notions of "deployment environment" were significantly different in both the meaning and in the additional attributes used to describe specific instances. This is to say I don't think we can reasonably generalize how different orgs model deployments.
  2. The use of the word "annotation" or the other variants indicates the same issue - we don't know what we're modeling with this dimension and therefore can't give it any meaningful name.
  3. The next time someone comes and says "we actually have 3 attributes per deployment", are we going to add "annotation2" dimension?

I think OTEL should be unopinionated on this topic and provide a general mechanism that is flexible to different use cases. Kubernetes solves this problems with tags - it's up to you how to name those tags and which values to give them. We could have an attribute deployment.tags which is itself a map<string, string>, with no further restrictions. This still gives the monitoring UIs a single place to look for deployment-related things.

thanks for taking the time to review, I have the same concerns as you on points 1-3, ie: hard to model, annotation is lazy data modeling, people might want 3,4 etc.

I'm very open to rewriting the proposal to propose using a tag attribute if that is something that OTEL would consider.

yurishkuro commented 2 years ago

One big downside of the ...tags attribute as a map is that it is a more complex embedded data structure that may not be indexed well by the existing backends. I wonder if a naming pattern like deployment.tags.{tagname} would be more useful, but I don't think we used this pattern in OTEL so far.

I have no immediate interest in this aspect of the modeling, so I suggest seeking feedback from the community.

tedsuo commented 1 year ago

@bodepd we are marking OTEPs which are no longer being updated as stale. We will close this PR in one week. If you wish to resurrect this PR, that is fine.