[processor/resourcedetection] New dimension host.id was added enabled by default

[atoulme]

Component(s)

processor/resourcedetection

What happened?

Description

With #18618, we added a new dimension host.id and it was marked enabled by default. Unfortunately, this conflicts with our own detectors and is causing issues.

We would like to make sure this dimension is not enabled by default and going forward that we make sure any data change is considered as not default to avoid surprises.

Collector version

0.75.0

Environment information

No response

OpenTelemetry Collector configuration

No response

Log output

No response

Additional context

No response

[github-actions]

Pinging code owners:

• processor/resourcedetection: @Aneurysm9 @dashpole

See Adding Labels via Comments if you do not have permissions to add labels yourself.

[mx-psi]

I would argue that adding things enabled by default has been the usual practice for the resource detection processor. Other examples can be seen in #12914 and #12355. I think we should not freeze the set of attributes added by a given detector by default until we deem the detector stable, and we have not done (to my knowledge) any analysis of whether the system detector or any others are stable enough.

I think a better option until such stability is reached is to use the functionality added in #8547 if you want to freeze the set of attributes added by the processor.

[atoulme]

@mx-psi this change caused inadvertent change and I think we ought to have some functional test coverage to make sure we understand how cascading detectors may inadvertently impact each other. We can cover that with a test and just make sure we report if the behavior has changed as a breaking change so folks downstream (👋 ) can do the right thing and not run into surprises.

[dmitryax]

I submitted #21482 which should help us to avoid issues like this going forward

[mx-psi]

make sure we report if the behavior has changed as a breaking change

That makes sense to me :)

[mx-psi]

Now that #23253 has landed, should we mark host.id as enabled: false on the system detector and mark it as a breaking change?

[mx-psi]

Filed #24010 for this

[sumo-drosiek]

this conflicts with our own detectors and is causing issues.

What does mean our own detectors? hostmetricsreceiver?

[mx-psi]

I am not sure what @atoulme meant, but I think an example where things could break for users is the following:

processors:
  resourcedetection:
    detectors: [system, ec2]

If we are on EC2, then:

1. Before v0.72.0, host.id would have the EC2 instance id
2. After v0.72.0, host.id would have the machine id (from e.g. /etc/machine-id)

Ultimately the issue here is that host.id has multiple meanings that are all valid on the same host. Before v0.72.0 one could use the presence of other attributes (e.g. cloud.provider or cloud.platform) to determine which of the two meanings (EC2 instance id or machine id) was the one set here, but v0.72.0 broke that. I feel like this is an issue at the spec level, but until we fix that at the spec restoring the old behavior sounds better to me.

[sumo-drosiek]

Shouldn't the solution be to disable host.id by default for all detectors?
It is easy to go into this conflict on the other way around just by reordering detectors

[mx-psi]

@sumo-drosiek My main point in having host.id enabled on ec2 but not in system is backwards compatibility, since that's the way it has been for a long time. I think it can be okay to have host.id disabled on every detector (I haven't thought too closely about it), but I would like to do that on a separate step if we go that way

[sumo-drosiek]

My main point in having host.id enabled on ec2 but not in system is backwards compatibility, since that's the way it has been for a long time.

Thanks for clarification 🙏