[extension/opamp] Make reported service.instance.id always match Collector resource attributes by jade-guiton-dd · Pull Request #46495 · open-telemetry/opentelemetry-collector-contrib

jade-guiton-dd · 2026-02-27T17:55:10Z

Description

This PR changes the OpAMP extension's behavior so that the reported service.instance.id in agent_description.identifying_attributes always matches the value of service.instance.id in the Collector's resource attributes, which was previously not always the case if the OpAMP instance_uid is manually set in the extension's configuration.

This (in my view) violated the OpAMP specification's recommendation that:

The Agent SHOULD also include these attributes in the Resource of its own telemetry. The combination of identifying attributes SHOULD be sufficient to uniquely identify the Agent’s own telemetry in the destination system to which the Agent sends its own telemetry.

Additionally, on Tigran's suggestion, the instance_uid is no longer based on the service.instance.id in the common case; from the release note:

Previously:

the service.instance.id reported in the AgentDescription was based on the OpAMP instance UID

the instance UID was typically set based on the service.instance.id from the Collector resource attributes

it could be overriden using the instance_uid configuration of the OpAMP extension

This meant that the reported service.instance.id did not always match the Collector resource attributes, which is a problem for correlation, and that server implementations got used to the typical case of service.instance.id and instance_uid matching, despite there being no guarantee of this.

Now:

the reported value of service.instance.id always matches the Collector resource attributes

the instance UID is either taken from the instance_uid configuration or generated randomly.

This means that the two values can never be expected to match, unless both configurations are explicitly set to the same value. That is what the OpAMP supervisor does, which means its behavior is unaffected.

…ributes

jade-guiton-dd · 2026-03-02T15:55:45Z

I managed to reproduce the supervisor-test failure locally once, and then never again... Not sure if it's flaky?

Edit: Managed to reproduce things. I was looking at the wrong test and needed to rebuild the otelcontribcol binary used in the test.

tigrannajaryan · 2026-03-09T14:56:08Z

-			if err != nil {
-				return nil, err
-			}
+	} else if agentInstanceID != "" {


I am not sure we want this behavior. I think the logic should be:

If InstanceUID is specified in the config then use it. This is needed for the Supervisor use case.

If InstanceUID is NOT specified in the config then generate a UUIDv7 and use it. We should not use the value of ServiceInstanceIDKey attribute as the value of OpAMP instance UID. ServiceInstanceIDKey is not guaranteed to be globally unique. If we want, we can additionally persist the value of generated UUIDv7, so that we reuse the value on restarts. That can be a new feature we can consider for opampextention.

Hm, that would be a much more impactful change for people using the extension without the supervisor, but I can see value in not getting server implementations used to both values matching.

I don't think the first part would be necessary since (if I'm not mistaken) the supervisor always sets both instance_uid and service.instance.id in the Collector resource attributes, so I guess it would just amount to removing this else branch entirely.

it would just amount to removing this else branch entirely.

yes, that's what I was suggesting.

@evan-bradley @andykellr what do you think?

douglascamata

I think the implementation looks okay, but I'm not sure if the tests cover all the possible scenarios now. From what I understand, I don't see a test where we have the instance UID configured and assert that the extension's service instance ID matches the Collector's.

douglascamata · 2026-03-10T14:07:00Z

 	expectedDescription := &protobufs.AgentDescription{
 		IdentifyingAttributes: []*protobufs.KeyValue{
 			stringKeyValue("client.id", "my-client-id"),
-			stringKeyValue("service.instance.id", uuid.UUID(ad.InstanceUid).String()),


Why is this removed? Looks weird. I think we should assert on some expected service instance ID here, no?

Hm, you're right. I removed it because the test was failing and I thought it was testing the extension (in which case we expect the instance ID to be random), but since this seems to be for the supervisor I would expect it to match the instance UID...

The usual error seems to be "could not get bootstrap info from the Collector: collector's OpAMP client never connected to the Supervisor", which I can't easily relate to any of my changes. I've been trying to debug these tests for a while but I'm honestly somewhat at a loss, mostly because all the useful logic happens in a different process, so the debugger never gets attached. Do you have advice on where the discrepancy could come from?

I tried some stuff locally and I hit a similar problem to you, @jade-guiton-dd. I could run only this specific test with go test -tags=e2e -run 'TestSupervisorAgentDescriptionConfigApplies' -count=1 -v . (in the cmd/opampsupervisor folder) though.

It seems like require.Subset ends up comparing values with reflect.DeepEqual (see https://github.com/stretchr/testify/blob/v1.11.1/assert/assertions.go#L71) and I think the protobuf types might somehow be interfering with the comparison now, but I don't understand why.

I did some local changes to convert []*protobufs.Keyvalue to a map[string]string and I got a proper test failure due to mismatch in expected values:

diff --git a/cmd/opampsupervisor/e2e_test.go b/cmd/opampsupervisor/e2e_test.go index 5efe2bc722c..8a0ce229c19 100644 --- a/cmd/opampsupervisor/e2e_test.go +++ b/cmd/opampsupervisor/e2e_test.go @@ -1237,6 +1237,7 @@ func TestSupervisorAgentDescriptionConfigApplies(t *testing.T) { expectedDescription := &protobufs.AgentDescription{ IdentifyingAttributes: []*protobufs.KeyValue{ stringKeyValue("client.id", "my-client-id"), + stringKeyValue("service.instance.id", uuid.UUID(ad.InstanceUid).String()), stringKeyValue("service.name", command), stringKeyValue("service.version", version), }, @@ -1248,8 +1249,14 @@ func TestSupervisorAgentDescriptionConfigApplies(t *testing.T) { }, } - require.Subset(t, ad.AgentDescription.IdentifyingAttributes, expectedDescription.IdentifyingAttributes) - require.Subset(t, ad.AgentDescription.NonIdentifyingAttributes, expectedDescription.NonIdentifyingAttributes) + actualIdentifyingAttributes := keyValuesToStringMap(ad.AgentDescription.IdentifyingAttributes) + require.Subset(t, actualIdentifyingAttributes, keyValuesToStringMap(expectedDescription.IdentifyingAttributes)) + require.Contains(t, actualIdentifyingAttributes, "service.instance.id") + _, err = uuid.Parse(actualIdentifyingAttributes["service.instance.id"]) + require.NoError(t, err) + + actualNonIdentifyingAttributes := keyValuesToStringMap(ad.AgentDescription.NonIdentifyingAttributes) + require.Subset(t, actualNonIdentifyingAttributes, keyValuesToStringMap(expectedDescription.NonIdentifyingAttributes)) time.Sleep(250 * time.Millisecond) } @@ -1265,6 +1272,14 @@ func stringKeyValue(key, val string) *protobufs.KeyValue { } } +func keyValuesToStringMap(kvs []*protobufs.KeyValue) map[string]string { + out := make(map[string]string, len(kvs)) + for _, kv := range kvs { + out[kv.Key] = kv.Value.GetStringValue() + } + return out +} + // Creates a Collector config that reads and writes logs to files and provides // file descriptors for I/O operations to those files. The files are placed // in a unique temp directory that is cleaned up after the test's completion.

When I run the test above with the assertion that this PR removed on the service.instance.id plus this patch applied I get this:

(...) e2e_test.go:1253: Error Trace: /Users/douglas/code/opentelemetry-collector-contrib/cmd/opampsupervisor/e2e_test.go:1253 Error: map[string]string{"client.id":"my-client-id", "service.instance.id":"6b1f4a09-081d-4d28-ae2c-85c59ab349fe", "service.name":"otelcontribcol", "service.version":"0.146.0-dev"} does not contain map[string]string{"client.id":"my-client-id", "service.instance.id":"019cd884-2d8a-72b3-b7ab-00a3a0eb0623", "service.name":"otelcontribcol", "service.version":"0.146.0-dev"} Test: TestSupervisorAgentDescriptionConfigApplies (...)

Right, so that confirms that the supervisor is not properly setting service.instance.id at the Collector level.

I originally assumed it did because of this template, but after looking deeper into it, I'm not actually sure how ResourceAttributes is filled. It definitely contains the explicitly-specified values from agent.description.identifying_attributes, but I'm not sure if service.instance.id is ever added in there.

So I guess there are two options:

either keep the existing behavior in the Supervisor where service.instance.id is always set to the OpAMP instance UID, which would require modifying the code to make sure the resource attribute is always overwritten

apply the same "radical" change Tigran proposed in the Supervisor as well, and completely decorrelate them (which amounts to removing a test case in the way I did above)

Since for some reason the test is still failing for me locally, I tried pushing a new version with a modified composeExtraTelemetryConfig that adds the test line back in and always sets service.instance.id to the instance UID (unless explicitly set in s.agentDescription)

Okay, I figured it out.

The reason why the tests were failing locally for me with that different error is that the bootstrap Collector tries to start its Prometheus endpoint, but I had a leftover Collector running on my system, causing a port conflict and preventing the bootstrap Collector from starting.

The reason why my latest change apparently failed to set service.instance.id is because composeExtraTelemetryConfig is not called when composing the configuration for the bootstrap Collector. So I instead decided to set the attribute directly inside templates/opampextension.yaml. This seems to fix the test, which is also simplified a bit from the patch you suggested above. I am assuming this will compose gracefully with the "extra telemetry" configuration.

Awesome. I'm happy that this is fixed and that you could use a simpler patch 😃

I will do a full review pass in a bit. Great work!

@douglascamata did you find the time to review this?

I’ll find some time to check it out later today or tomorrow. 👍

…t during bootstrap + fix TestSupervisorAgentDescriptionConfigApplies

…bute

github-actions · 2026-03-26T05:47:52Z

This PR was marked stale due to lack of activity. It will be closed in 14 days.

tigrannajaryan · 2026-03-26T13:02:57Z

-	resourceAttrs map[string]string
+	agentType       string
+	agentVersion    string
+	agentInstanceID string


I am confused by why we have 2 fields now agentInstanceID and instanceID. What's the difference other than the field type? I expect the agent to have one instance id. Can these fields have 2 different id values? When does that happen?

From what I understood through the changelog entry, agentInstanceID represents the Collector's instance ID and instanceID represents the OpAMP extension's instance_uid. Both will match unless instance_uid is explicitly configured in the opampextension. Please correct me if I'm wrong, @jade-guiton-dd.

It sounds a bit confusing, yes. Maybe we could have a comment explaining why these two fields with very similar names exist?

The two concepts have always had confusingly similar names to be honest, but I can rename the fields and add comments if that makes things clearer.

Note that the change Tigran suggested earlier makes it so that they won't usually match. They will only match if the user (or the supervisor) manually sets both (with the instance_uid config and service::telemetry::resource) to the same specific value.

I ended up renaming agentType, agentVersion, agentInstanceID, and instanceID to serviceName, serviceVersion, serviceInstanceID, and instanceUID respectively, tell me if that helps clear things up.

OK, renaming definitely helps understand what's going on.

I think this is the desirable behavior now.

Can you please also describe in a bit more details in the PR description the impact of the change. The description you have in changelog subtext is great, I think it can be used in the PR description too.

douglascamata

Even though I have some small suggestions and there are some questions/feedback from others, I don't have further concerns about this.

tigrannajaryan · 2026-03-27T13:32:44Z

@evan-bradley @andykellr can you please also take a look. I would like your confirmation on this change.

github-actions · 2026-04-11T05:41:43Z

This PR was marked stale due to lack of activity. It will be closed in 14 days.

jade-guiton-dd · 2026-04-13T10:15:52Z

@evan-bradley @andykellr Could we get a review here? 🙏

jade-guiton-dd added 3 commits February 27, 2026 18:51

Make reported service.instance.id always match Collector resource att…

729fef4

…ributes

Fix error when service.instance.id isn't set (usually tests)

e42a245

Add PR number to changelog

184d973

Update supervisor to check instance UID instead of service.instance.id

6e8a8d1

jade-guiton-dd mentioned this pull request Mar 4, 2026

[extension/opamp] Reported identifying attributes not matching Collector internal telemetry #46649

Open

tigrannajaryan reviewed Mar 9, 2026

View reviewed changes

jade-guiton-dd added 3 commits March 10, 2026 10:53

instance_uid is no longer taken from service.instance.id

dcdef68

Fix TestNewOpampAgentAttributes

23d4123

Fix TestSupervisorAgentDescriptionConfigApplies

3800815

jade-guiton-dd marked this pull request as ready for review March 10, 2026 13:25

jade-guiton-dd requested review from a team, atoulme and evan-bradley as code owners March 10, 2026 13:25

github-actions bot assigned bogdandrutu Mar 10, 2026

github-actions bot added extension/opamp cmd/opampsupervisor labels Mar 10, 2026

github-actions bot requested a review from portertech March 10, 2026 13:25

douglascamata reviewed Mar 10, 2026

View reviewed changes

jade-guiton-dd added 3 commits March 10, 2026 17:48

Try explicitly setting service.instance.id in Supervisor

2ab8dff

Set service.instance.id in opampextension.yaml instead so it's presen…

3d79558

…t during bootstrap + fix TestSupervisorAgentDescriptionConfigApplies

Adjust tests that don't expect the service.instance.id resource attri…

cbc4513

…bute

github-actions bot added the Stale label Mar 26, 2026

tigrannajaryan reviewed Mar 26, 2026

View reviewed changes

github-actions bot removed the Stale label Mar 27, 2026

douglascamata approved these changes Mar 27, 2026

View reviewed changes

Rename opampAgent fields for clarity

de9055a

tigrannajaryan approved these changes Mar 27, 2026

View reviewed changes

github-actions bot added the Stale label Apr 11, 2026

github-actions bot removed the Stale label Apr 14, 2026

andykellr approved these changes Apr 20, 2026

View reviewed changes

Conversation

jade-guiton-dd commented Feb 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Uh oh!

jade-guiton-dd commented Mar 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

douglascamata left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jade-guiton-dd Mar 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

douglascamata Mar 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jade-guiton-dd Mar 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jade-guiton-dd Mar 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jade-guiton-dd Mar 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Mar 26, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jade-guiton-dd Mar 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

douglascamata left a comment

Choose a reason for hiding this comment

Uh oh!

tigrannajaryan commented Mar 27, 2026

Uh oh!

github-actions bot commented Apr 11, 2026

Uh oh!

jade-guiton-dd commented Apr 13, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

jade-guiton-dd commented Feb 27, 2026 •

edited

Loading

jade-guiton-dd commented Mar 2, 2026 •

edited

Loading

jade-guiton-dd Mar 10, 2026 •

edited

Loading

douglascamata Mar 10, 2026 •

edited

Loading

jade-guiton-dd Mar 10, 2026 •

edited

Loading

jade-guiton-dd Mar 10, 2026 •

edited

Loading

jade-guiton-dd Mar 11, 2026 •

edited

Loading

jade-guiton-dd Mar 27, 2026 •

edited

Loading