Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make Resource.Merge consistent with Span.SetAttribute. #1345

Merged
merged 5 commits into from
Jan 20, 2021
Merged

Make Resource.Merge consistent with Span.SetAttribute. #1345

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
ea509c7
Make Resource.Merge consistent with Span.SetAttribute.
Oberon00 Jan 15, 2021
5c3fcb5
Update CHANGELOG, compliance-matrix.
Oberon00 Jan 15, 2021
39fafa5
Fill in PR#.
Oberon00 Jan 15, 2021
7963e98
Update specification/resource/sdk.md
Oberon00 Jan 15, 2021
c44e9df
Merge branch 'master' into resource-merge-reverse
Oberon00 Jan 20, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,9 @@ Updates:
([#1322](https://github.com/open-telemetry/opentelemetry-specification/pull/1322))
- Require schemed endpoints for OTLP exporters
([1234](https://github.com/open-telemetry/opentelemetry-specification/pull/1234))
- Resource SDK: Reverse (suggested) order of Resource.Merge parameters, remove
special case for empty strings
([#1345](https://github.com/open-telemetry/opentelemetry-specification/pull/1345))

## v0.7.0 (11-18-2020)

Expand Down
2 changes: 1 addition & 1 deletion spec-compliance-matrix.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ status of the feature is not known.
|----------------------------------------------|---|----|---|------|----|------|---|----|---|----|-----|
|Create from Attributes | + | + | + | + | + | + | | + | | + | + |
|Create empty | + | + | + | + | + | + | | + | | + | + |
|Merge | + | + | + | + | + | + | | + | | + | + |
|[Merge (v2)](specification/resource/sdk.md#merge) | | | | | | | | | | | |
|Retrieve attributes | + | + | + | + | + | + | | + | | + | + |
|[Default value](specification/resource/semantic_conventions/README.md#attributes-with-default-value) for service.name | | | | | | | | | | | |

Expand Down
18 changes: 6 additions & 12 deletions specification/resource/sdk.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,27 +43,21 @@ Required parameters:

### Merge

The interface MUST provide a way for a primary resource and a
secondary resource to be merged into a new resource.
The interface MUST provide a way for a old resource and a
Oberon00 marked this conversation as resolved.
Show resolved Hide resolved
updating resource to be merged into a new resource.

Note: This is intended to be utilized for merging of resources whose attributes
come from different sources,
such as environment variables, or metadata extracted from the host or container.

The resulting resource MUST have all attributes that are on any of the two input resources.
Conflicts (i.e. a key for which attributes exist on both the primary and secondary resource)
MUST be handled as follows:

* If the value on the primary resource is an empty string, the result has the value of the secondary resource.
* Otherwise, the value of the primary resource is used.

Attribute key namespacing SHOULD be used to prevent collisions across different
resource detection steps.
If a key exists on both the old and updating resource, the value of the updating
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
If a key exists on both the old and updating resource, the value of the updating
If a key exists on both the existing and updating resource, the value of the updating

resource MUST be picked (even if the updated value is empty).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we sure about this?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

About the behavior for empty strings? I hope so, it is one special case we lose.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ping @bogdandrutu ;)


Required parameters:

- the primary resource whose attributes take precedence.
- the secondary resource whose attributes will be merged in.
- the old resource
- the updating resource whose attributes take precedence

### The empty resource

Expand Down