service.instance.id implementation

[zeitlinger]

Implements service.instance.id spec PR

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 91.09%. Comparing base (98eded9) to head (62a2b6d).

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #6226      +/-   ##
============================================
+ Coverage     91.07%   91.09%   +0.01%     
- Complexity     5751     5757       +6     
============================================
  Files           626      627       +1     
  Lines         16782    16790       +8     
  Branches       1718     1718              
============================================
+ Hits          15285    15295      +10     
+ Misses         1003     1002       -1     
+ Partials        494      493       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[zeitlinger]

Updated the PR to reflect the spec PR as it was merged.

[jack-berg]

Couple of nits but looks good to me.

@open-telemetry/java-approvers this PR implements a recent change merged to semantic conventions. It has not bee formally released, and the service.instance.id attribute is still marked as experimental. So there's a valid question of whether we should include this attribute by default or not.

If we're nervous about merging this until stable, we could include it in the sdk extension incubator project instead. The inclusion of the incubator on the classpath acting as an "opt in" from the user.

[jpkrohling]

I agree that a release is desirable before merging this: until it's released, changes are always possible, even though they are unlikely for this specific case.

About it being experimental: I believe this change isn't causing backward compatibility issues, other than possibly for cardinality concerns when it comes to metrics. If possible, I'd love this to be opt-out, as it would help us make that semconv stable within a few releases.