Add a resource detector for populating service.instance.id

[pyohannes]

Description

This adds a resource detector for populating service.instance.id in accordance with semantic conventions.

Currently this resource detector is not enabled by default, however, once semantic conventions demand this attribute to be present by default (which is planned), then this detector can easily be added as a default (similar to the otel detector).

Fixes #2113

Type of change

Please delete options that are not relevant.

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• This change requires a documentation update

How Has This Been Tested?

This is tested via unit tests

Checklist:

• Followed the style guidelines of this project
• Changelogs have been updated
• Unit tests have been added
• Documentation has been updated

[xrmx]

Note to self: need to run a gunicorn app with this to understand how it will behaves regarding Resources lifetimes

[xrmx]

Note to self: need to run a gunicorn app with this to understand how it will behaves regarding Resources lifetimes

Tried:

OTEL_EXPERIMENTAL_RESOURCE_DETECTORS=otel,serviceinstanceid,process opentelemetry-instrument --traces_exporter console --metrics_exporter none --logs_exporter none --service_name foo gunicorn app:app -w2

where app is the flask dice app from the opentelemetry.io documentation and out of the box without a post_fork hook configured, gunicorn is reporting the same service.instance.id for both workers. Same problem with the process resource detector, we get the same pid on the traces but response is from different workers.

[lzchen]

Any reason not to simply wait until service.instance.id get added as stable and include them in OtelResourceDetector? Are there plans to include them as part of "Semantic Attributes with SDK-provided Default Value"

[pyohannes]

Are there plans to include them as part of "Semantic Attributes with SDK-provided Default Value"

Yes, see open-telemetry/semantic-conventions#311.

Any reason not to simply wait until service.instance.id get added as stable and include them in OtelResourceDetector?

One prerequisite for adding service.instance.id as stable are working prototypes and implementation in several languages. Having such an implementation (or at least prototype) in Python would be beneficial for those stabilization efforts, especially as the description of service.instance.id explicitly mentions application servers like gunicorn.

Showing that service.instance.id can be populated according to the conventions for all major usage scenarios in Python would be a prerequisite for declaring it stable and making it a default attribute.

[lzchen]

Yes, see open-telemetry/semantic-conventions#311.

Having such an implementation (or at least prototype) in Python would be beneficial for those stabilization efforts

If there are plans to make them default doesn't that mean they would move into OtelResourceDetector eventually? If we create a new resource detector (even though it might remain experimental) it sounds like we would have to support this forever. Maybe I might be misunderstanding what "prototype" is, will this just be temporary to get the specs to move along?

[pyohannes]

Note to self: need to run a gunicorn app with this to understand how it will behaves regarding Resources lifetimes

I'll give this a try myself.

[xrmx]

Note to self: need to run a gunicorn app with this to understand how it will behaves regarding Resources lifetimes

I'll give this a try myself.

See #4061 (comment), the Resources are instantiated in the gunicorn process manager during distro bootstrap so they would be the same for every worker.

So to make this less miserable for users we could find a safe way to redo part of the bootstrap after fork, currently we don't permit this, with:

def post_fork(server, worker):
    from opentelemetry.instrumentation.auto_instrumentation.sitecustomize import _load_configurators
    _load_configurators()

You'll get:

Overriding of current TracerProvider is not allowed
Overriding of current MeterProvider is not allowed

Removing the set once check will make it work fine though:

diff --git a/opentelemetry-api/src/opentelemetry/metrics/_internal/__init__.py b/opentelemetry-api/src/opentelemetry/metrics/_internal/__init__.py
index 972ff270..a77144e0 100644
--- a/opentelemetry-api/src/opentelemetry/metrics/_internal/__init__.py
+++ b/opentelemetry-api/src/opentelemetry/metrics/_internal/__init__.py
@@ -786,6 +786,8 @@ def _set_meter_provider(meter_provider: MeterProvider, log: bool) -> None:
         # gives all proxies real instruments off the newly set meter provider
         _PROXY_METER_PROVIDER.on_set_meter_provider(meter_provider)
 
+    set_mp()
+    return
     did_set = _METER_PROVIDER_SET_ONCE.do_once(set_mp)
 
     if log and not did_set:
diff --git a/opentelemetry-api/src/opentelemetry/trace/__init__.py b/opentelemetry-api/src/opentelemetry/trace/__init__.py
index 28300f40..5816a216 100644
--- a/opentelemetry-api/src/opentelemetry/trace/__init__.py
+++ b/opentelemetry-api/src/opentelemetry/trace/__init__.py
@@ -512,6 +512,8 @@ def _set_tracer_provider(tracer_provider: TracerProvider, log: bool) -> None:
         global _TRACER_PROVIDER  # pylint: disable=global-statement
         _TRACER_PROVIDER = tracer_provider
 
+    set_tp()
+    return
     did_set = _TRACER_PROVIDER_SET_ONCE.do_once(set_tp)
 
     if log and not did_set: