donbourne
commented
3 months ago @edburns , your changes in this PR look good to me and IMO would be fine to be in the next release. I'll wait for our meetings to resume in September to discuss with others on the MP Telemetry call.
Regarding your comment in the vote thread...
I have a question. This question pertains in both SemConv v1.27.0 and v1.26.0, so it is relevant to MP 2.0:
Consider this text from: https://opentelemetry.io/docs/specs/semconv/general/attribute-requirement-level/#conditionally-required
When a Conditionally Required attribute’s condition is not satisfied, and there is no requirement to populate the attribute, semantic conventions MAY provide special instructions on how to handle it. If no instructions are given and if instrumentation can populate the attribute, instrumentation SHOULD use the Opt-In requirement level on the attribute.
I wonder if MP telemetry should say something about what MP telemetry implementations should do in this case? Not required but just curious. Maybe forward this on to Don Bourne?
I can see that our guidance in the MP Telemetry spec doesn't cover all cases. Our spec says "All attributes that are listed as conditionally required and stable in the OpenTelemetry Semantic Conventions MUST be included when the per-attribute condition described in the OpenTelemetry Semantic Conventions is satisfied." -- which I realize doesn't address specifically what to do when the per-attribute condition is not satisfied. Our intent is that people follow the guidance from the semantic convention for conditional attributes (not just the guidance when the condition is satisfied), so that could be better worded. If you want to open an issue for that or add text for that to this PR, we can discuss a good resolution.
Emily-Jiang
modified: spec/src/main/asciidoc/metrics.adoc
Two trivial changes.
Clarify that conditions are per-attribute. Might not be obvious to someone not deeply familiar with SemConv.
SemConv uses
Opt-In. I corrected the case.