Fix Resource dedup logic

[BrynCooke]

Changes

Previously Resource would have last k,v wins when supplied a list of k,v even if it was empty.

This change brings the logic in line with the documentation on Resource::new and also applies the same to from_detectors.

Design

Suspect that the implementation was just an oversight as the existing doc comment seems to describe the desired behaviour.

Users will be able to specify a list of detectors in priority order and expect them to have first non empty value wins.

Merge requirement checklist

• CONTRIBUTING guidelines followed
• Unit tests added/updated (if applicable)
• Appropriate CHANGELOG.md files updated for non-trivial, user-facing changes
• Changes in public API reviewed (if applicable)

[codecov]

Codecov Report

All modified lines are covered by tests ✅

Files	Coverage Δ
opentelemetry-sdk/src/resource/mod.rs	98.7% <100.0%> (+0.2%)	⬆️

📢 Thoughts on this report? Let us know!.

[hdost]

@BrynCooke thank you for your contribution. Unfortunately looking at the specs we should not even have an insert https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/resource/sdk.md

[BrynCooke]

@hdost The insert method is private, it's not part of the public API, just to keep things dry.

[hdost]

@hdost The insert method is private, it's not part of the public API, just to keep things dry.

Indeed, however I do have one other issue as this works counter to the spec for merge.

The resulting resource MUST have all attributes that are on any of the two input resources. If a key exists on both the old and updating resource, the value of the updating resource MUST be picked (even if the updated value is empty).

[BrynCooke]

Makes sense, I've instead raise an issue for discussion #1298 that hopefully captures the issues around the current behaviour. I'll leave this PR for you to close though.

[lalitb]

Indeed, however I do have one other issue as this works counter to the spec for merge.

Good point.

[BrynCooke]

Also from the spec:
| Resource detector packages MAY detect resource information from multiple possible sources and merge the result using the Merge operation described above.

So definitely agree that this PR doesn't fit the bill.

deviates from merge logic