Add service.instance.id with Pod name

[janario]

Description:

I'm using newrelic with opentelemetry and according to their documentation they rely on the service.instance.id to group jvm metrics.

https://opentelemetry.io/docs/specs/semconv/resource/#service-experimental

After a few explorations, I noticed that this was expected here open-telemetry/semantic-conventions#312

But while the pod is being created it may not have a name yet, which results on an absent property.

This PR will change it to be more dynamic at runtime.

Link to tracking Issue(s):

• Resolves: service.instance.id is not added because of empty podName #2679

Testing:

Documentation:

https://github.com/open-telemetry/semantic-conventions/blob/main/docs/attributes-registry/service.md

[yuriolisa]

Could you please submit some unit tests for this PR?

[pavolloffay]

It would be nice to see this in e2e tests

[pavolloffay]

Could you please include it in the changelog?

[janario]

I have a few more internal details that I'm validating but I plan to get back on this soon, maybe this or next week I hope

[janario]

Marking this as a draft, I plan to get back on this and a few other issues soon after some internal migration.

[swiatekm]

The change itself looks good to me. For consistency, I think this logic should be moved into the injectCommonSDKConfig function, where we handle these kinds of cases. I'd also like to see a unit test checking it.

[janario]

The change itself looks good to me. For consistency, I think this logic should be moved into the injectCommonSDKConfig function, where we handle these kinds of cases. I'd also like to see a unit test checking it.

@swiatekm-sumo

hum moving would be a bit hard I guess since it would refac a few things 🤔

About the test, I've fixed/adjusted the other tests to consider the new attributes I guess in some ways they cover it already

Do you see some other test/adjustment that should be done?

[janario]

@yuriolisa @pavolloffay @swiatekm-sumo

I'll create the changelog soon.
Can you pls give it another review and share your opinion on this? #2678 (comment)

So after the final change I close with the changelog.

[jaronoff97]

I think this looks good... @janario testing locally could you send some screenshots of new relic (or something else) showing this working as intended?

[janario]

I think this looks good... @janario testing locally could you send some screenshots of new relic (or something else) showing this working as intended?

Sure,

By default without the attribute it doesn't show the JVM view of a service:

After having it starts to work:

---

In the end we were under evaluation between Newrelic and other options, and we choose for another option :-)
But I think it still worth to be fixed.

[janario]

@pavolloffay did you have a chance to review this one? :-)

[janario]

hi
Any updates here?