Add schema_url to Resource #1871

dgetu · 2021-05-25T22:27:43Z

A change in the OTel specification requires a new schema_url field in the Resource class. This change implements the field, updates the Resource factory method and merge method, updates the behavior of OTELResourceDetector when aggregating Resources, and modifies tests which used the old constructor signature.

Type of change

New feature (non-breaking change which adds functionality)
This change requires a documentation update

How Has This Been Tested?

All tox tests were ran on Python 3.9. Three tests were made or substantively modified, with a number of others modified for failing to conform to the new constructor signature.

Does This PR Require a Contrib Repo Change?

The OTel specification has changed which prompted this PR to update the method interfaces of opentelemetry-api/ or opentelemetry-sdk/
Yes.
No.

Checklist:

Followed the style guidelines of this project
Changelogs have been updated
Unit tests have been added
Documentation has been updated

…ibute

Co-authored-by: Eddy Lin <[email protected]>

…ibute

Co-authored-by: Eddy Lin <[email protected]>

…elemetry-python into add-schema_url-attribute

Improves readability from the last commit

This reverts commit 2801745.

linux-foundation-easycla · 2021-05-25T22:27:53Z

The committers are authorized under a signed CLA.

✅ Daniel Getu (b93518f, f5246af, e1ca938, db3cee0, 222ab97, 6e1219d, bac41bb, 752611a, 8817527, 1b03a5c, 8d7f8d1, 99af3e7, da1cbe2, d3afaff, 20aec16, 602600f, 168f24f, 7a0c286, b7450eb, 5af86c5, 00dac38, fca2614, 5e1a565, 32ce848, 2801745, 3debbee, f938458)

codeboten

Thanks for taking this on, making the schema_url an optional argument would prevent the need to change all the calls to it in the tests. It will also prevent this from being a breaking change for users.

opentelemetry-sdk/src/opentelemetry/sdk/resources/__init__.py

…ibute

…dd-schema_url-attribute

opentelemetry-sdk/src/opentelemetry/sdk/resources/__init__.py

Adds a test to verify the behavior of aggregating ResourceDetectors with different schema URLs. Verifies that merge error logs contain both conflicting URLs.

codeboten

Nice I think this is getting pretty close, please take a look at my comments and update the branch!

opentelemetry-sdk/tests/trace/test_trace.py

opentelemetry-sdk/src/opentelemetry/sdk/resources/__init__.py

dgetu and others added 26 commits May 21, 2021 14:25

Add Resources.schema_url to Resource.create test

b93518f

Add Resources.schema_url to Resource.merge test

f5246af

Implement schema_url field in Resource

e1ca938

Fix broken unrelated tests

db3cee0

Merge remote-tracking branch 'upstream/main' into add-schema_url-attr…

222ab97

…ibute

Add schema_url change to changelog

6e1219d

Update documentation

bac41bb

Change changelog entry to match existing style

752611a

Co-authored-by: Eddy Lin <[email protected]>

Merge remote-tracking branch 'origin/main' into add-schema_url-attribute

8817527

Remove assert statement

1b03a5c

Add Resource.schema_url to dunder eq and hash

8d7f8d1

Change Resource.merge to log an error instead of throwing an exception

99af3e7

Change format of parameters for readability

da1cbe2

Co-authored-by: Eddy Lin <[email protected]>

Fix linting issues

d3afaff

Merge remote-tracking branch 'upstream/main' into add-schema_url-attr…

20aec16

…ibute

Fix merge

602600f

Typo

168f24f

Co-authored-by: Eddy Lin <[email protected]>

Add schema_url to immutability test

7a0c286

Implement small requested changes

b7450eb

Merge branch 'add-schema_url-attribute' of github.com:open-o11y/opent…

5af86c5

…elemetry-python into add-schema_url-attribute

Update Resource.merge to return an empty resource when an error occurs

00dac38

Add rejection of combined detectors with differing Schema URLs

fca2614

Indicate that Changelog is WIP

5e1a565

Change Resource dunder hash to reduce collisions

32ce848

Change Resource dunder hash to reduce collisions

2801745

Improves readability from the last commit

Revert "Change Resource dunder hash to reduce collisions"

3debbee

This reverts commit 2801745.

dgetu mentioned this pull request May 25, 2021

Add schema_url to Resource open-telemetry/opentelemetry-python-contrib#511

Closed

8 tasks

Update changelog with PR

f938458

dgetu marked this pull request as ready for review May 25, 2021 22:48

dgetu requested review from a team, codeboten and ocelotl and removed request for a team May 25, 2021 22:48

Merge branch 'open-telemetry:main' into add-schema_url-attribute

0c27f4b

codeboten suggested changes May 26, 2021

View reviewed changes

opentelemetry-sdk/src/opentelemetry/sdk/resources/__init__.py Outdated Show resolved Hide resolved

dgetu added 4 commits May 26, 2021 10:53

Change Resource constructor to take optional parameter

1acf702

Merge remote-tracking branch 'origin' into add-schema_url-attribute

31f7bd6

Merge remote-tracking branch 'upstream/main' into add-schema_url-attr…

c6219f2

…ibute

Merge remote-tracking branch 'origin/add-schema_url-attribute' into a…

b087ca7

…dd-schema_url-attribute

dgetu requested a review from codeboten May 26, 2021 18:09

lzchen reviewed May 26, 2021

View reviewed changes

opentelemetry-sdk/src/opentelemetry/sdk/resources/__init__.py Outdated Show resolved Hide resolved

lzchen reviewed May 26, 2021

View reviewed changes

opentelemetry-sdk/src/opentelemetry/sdk/resources/__init__.py Outdated Show resolved Hide resolved

lzchen reviewed May 26, 2021

View reviewed changes

opentelemetry-sdk/src/opentelemetry/sdk/resources/__init__.py Outdated Show resolved Hide resolved

lzchen reviewed May 26, 2021

View reviewed changes

opentelemetry-sdk/src/opentelemetry/sdk/resources/__init__.py Outdated Show resolved Hide resolved

dgetu added 6 commits May 26, 2021 13:37

Implement small changes

f7e628d

Change Resource.merge to return old resource on error

b836134

Relegate ResourceDetector error checking to Resource.merge

360dc73

Update tests for aggregating ResourceDetectors

6bbbc08

Adds a test to verify the behavior of aggregating ResourceDetectors with different schema URLs. Verifies that merge error logs contain both conflicting URLs.

Update Resource.merge comment

da4bac1

Fix linting error

09c1bf3

dgetu requested a review from lzchen May 27, 2021 18:29

lzchen approved these changes May 27, 2021

View reviewed changes

codeboten reviewed May 27, 2021

View reviewed changes

opentelemetry-sdk/tests/trace/test_trace.py Outdated Show resolved Hide resolved

opentelemetry-sdk/src/opentelemetry/sdk/resources/__init__.py Outdated Show resolved Hide resolved

dgetu added 2 commits May 27, 2021 16:54

Fix typo

d218aad

Remove redundant null check

e84eedb

codeboten approved these changes May 28, 2021

View reviewed changes

lzchen merged commit 56495ed into open-telemetry:main Jun 1, 2021

dgetu mentioned this pull request Jun 2, 2021

REQUEST: New membership for dgetu open-telemetry/community#745

Closed

6 tasks

srikanthccv mentioned this pull request Jun 26, 2021

Add schema_url to resources #1862

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add schema_url to Resource #1871

Add schema_url to Resource #1871

dgetu commented May 25, 2021 •

edited

Loading

linux-foundation-easycla bot commented May 25, 2021 •

edited

Loading

codeboten left a comment

codeboten left a comment

Add schema_url to Resource #1871

Add schema_url to Resource #1871

Conversation

dgetu commented May 25, 2021 • edited Loading

Type of change

How Has This Been Tested?

Does This PR Require a Contrib Repo Change?

Checklist:

linux-foundation-easycla bot commented May 25, 2021 • edited Loading

codeboten left a comment

Choose a reason for hiding this comment

codeboten left a comment

Choose a reason for hiding this comment

dgetu commented May 25, 2021 •

edited

Loading

linux-foundation-easycla bot commented May 25, 2021 •

edited

Loading