Review semantic conventions specifications

[codeboten]

Part of the work for GA is to review semantic conventions spec and ensure our implementation is correct

[aabmass]

tick things off as separate issues are created

• There is no default resource detector to populate standard resource names not required
• Resources
  • telemetry.sdk.*, process.*, os.* - Populate resource attributes as per semantic conventions #1053
  • service.* - most users are instrumenting "services" so there should be an easy way to pass these in?
  • container.* should we have official resource detectors for popular container engines like Docker? can be done in an optional resource detector package, not needed for GA AFAIK
  • k8s.* should we have an official resource detector for Kubernetes? can be done in an optional resource detector package, not needed for GA AFAIK
• The system metrics package does not follow Standard system metrics and semantic conventions oteps#119 Update opentelemetry-instrumentation-system-metrics #1006
• Consider defining a library of semantic convention constants, similar to what's done in the collector Generate semantic convention constants/classes #1022
• Span name updated to follow semantic conventions to reduce cardinality #972

[codeboten]

* [ ]  There is no default resource detector to populate [standard resource names](https://github.com/open-telemetry/opentelemetry-specification/tree/master/specification/resource/semantic_conventions) (not sure if this is in scope)

Any thoughts on whether the telemetry.sdk.* values should be set in the Resource itself or via a ResourceDetector?

[aabmass]

I would say a resource detector because to keep the empty resource around. We already have this

opentelemetry-python/opentelemetry-sdk/src/opentelemetry/sdk/resources/__init__.py

Lines 74 to 76 in c42c5f3

	class OTELResourceDetector(ResourceDetector):
	# pylint: disable=no-self-use
	def detect(self) -> "Resource":

But what is the story for including these required resources labels, without any configuration?

[codeboten]

Not sure about the other attributes, but at least in the telemetry.sdk.* case they could be set in the OTelResourceDetector to

telemetry.sdk.name=opentelemetry
telemetry.sdk.language=python
telemetry.sdk.name=pkg_resources.get_distribution(
    "opentelemetry-sdk"
).version

[aabmass]

After discussing with some others, I think these telemetry properties should be set in the Tracer/Meter providers and not a detector, so that they are automatic.

I also missed this part of the resource semantic conventions:

Certain attribute groups in this document have a Required column. For these groups if any attribute from the particular group is present in the Resource then all attributes that are marked as Required MUST be also present in the Resource. However it is also valid if the entire attribute group is omitted (i.e. none of the attributes from the particular group are present even though some of them are marked as Required in this document).

So the "required fields" are only required if any of service.* are set.

[lzchen]

@aabmass
Curious, if we were to go the detector route, we would be instantiating a default resource detector which would probably be global correct, which after would automatically create a Resource with those "standard resources" and passed in whenever Tracer/Meter providers are instantiated. I guess the benefit of this would be we can define all the "standard" stuff in one place, where as setting in both Tracer/Meter we would have to make sure they are consistent (maybe just a function?)

[aabmass]

@lzchen From how the resource detection OTEP is written, I don't think the resource detector is global. It's just an interface and the user uses one or more of them at startup to create a merged resource object. It does talk about a default resource detector, but I think James is thinking of changing this when he actually writes the spec.

cc @james-bebbington who worked on the OTEP and is starting to write the spec. He suggested yesterday doing it right in the providers.

[aabmass]

Ok, I think the only thing remaining here is these service.* resource labels. My understanding now is that, if the user is instrumenting a service, they should pass these labels (although it's not strictly required) to indicate that it is a service. Should we have code to make this easy for the user? I don't think many instrumenting users would read the spec and decide to pass it on their own. Maybe this should be brought up in the spec repo?

[codeboten]

Closing this issue, will create separate issues as needed.