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.

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

Merge OTELResourceDetector result when creating resources #1096

Merged
merged 6 commits into from
Sep 18, 2020

Conversation

aabmass
Copy link
Member

@aabmass aabmass commented Sep 11, 2020

Description

Merge in resources detected from OTELResourceDetector (which parses OTEL_RESOURCE_ATTRIBUTES) when a resource is created. Here is the relevant section in the spec. I considered doing this in the module level declaration of _DEFAULT_RESOURCE, but then I'm not sure how to test this with mocking since it runs at import time.

Fixes #1063

Type of change

Please delete options that are not relevant.

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

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

  • updated test_resources.py

Checklist:

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

@aabmass aabmass changed the title Merge OTELResourceDetector result in Resource.create() Merge OTELResourceDetector result when creating resources Sep 11, 2020
@aabmass aabmass marked this pull request as ready for review September 11, 2020 01:38
@aabmass aabmass requested a review from a team September 11, 2020 01:38
@lzchen
Copy link
Contributor

lzchen commented Sep 13, 2020

TODO: mention this environment variable in the docs

Wouldn't this be a simple addition to this PR?

@aabmass
Copy link
Member Author

aabmass commented Sep 14, 2020

Wouldn't this be a simple addition to this PR?

Yup it was a note to myself but i forgot, will add it to this PR

@codeboten codeboten added the release:required-for-ga To be resolved before GA release label Sep 17, 2020
Copy link
Contributor

@codeboten codeboten left a comment

Choose a reason for hiding this comment

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

LGTM

@lzchen lzchen merged commit 497b322 into open-telemetry:master Sep 18, 2020
@aabmass aabmass deleted the detect-from-env branch September 21, 2020 20:08
alertedsnake pushed a commit to alertedsnake/opentelemetry-python that referenced this pull request Sep 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release:required-for-ga To be resolved before GA release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Align SDK with resource detection spec
3 participants