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

Jump to bottom

Add option for additional resource attributes with regex to trace exporter #145

Merged
merged 10 commits into from
Jun 2, 2021
Merged

Add option for additional resource attributes with regex to trace exporter #145

merged 10 commits into from
Jun 2, 2021

Conversation

dhpollack
Copy link
Contributor

@dhpollack dhpollack commented May 19, 2021 •
edited

The OpenTelemetry Specification allows one to specify additional attributes with the special environmental variable OTEL_RESOURCE_ATTRIBUTES. This includes those attributes into the span exported to GCP.

The goal of this PR is to easy add "global" attributes to all spans in a given service so that they can be filtered easily in GCP. But also it is not intuitive that you can add these attributes to spans, but they don't get exported with the CloudTraceSpanExporter.

Signed-off-by: David Pollack david@da3.net

@google-cla google-cla bot added the cla: yes label May 19, 2021
@dhpollack dhpollack marked this pull request as ready for review May 19, 2021 23:45
@dhpollack dhpollack mentioned this pull request May 19, 2021
Closed
@jsuereth jsuereth added the enhancement New feature or request label May 24, 2021
@aabmass
Copy link
Collaborator

aabmass commented May 24, 2021

Can you remove the OTEL_RESOURCE_ATTRIBUTES parts since that is already covered by OTel python? We are thinking the other part of this PR would be great to add, with some configuration. Can you update the CloudTraceSpanExporter constructor to accept a regex for the resource attributes to pass through. If the option isn't given, it shouldn't change the existing behavior

dhpollack and others added 2 commits May 24, 2021 23:02
@dhpollack
Add OTEL_RESOURCE_ATTRIBUTES to exporter
893f3a2 
The [OpenTelemetry
Specification](https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/resource/sdk.md#specifying-resource-information-via-an-environment-variable) allows one to specify additional attributes with the special environmental variable OTEL_RESOURCE_ATTRIBUTES.  This includes those attributes into the span exported to GCP.

Signed-off-by: David Pollack <david@da3.net>
Refactor to use OTELResourceDetector
377fdbe
@dhpollack dhpollack force-pushed the dhp/add-otel-resource-attributes branch from ed00f41 to 377fdbe Compare May 24, 2021 22:18
@aabmass
Copy link
Collaborator

aabmass commented May 24, 2021 •
edited

/gcbrun

@dhpollack
Copy link
Contributor Author

@aabmass I think this is ready now

aabmass
aabmass reviewed Jun 1, 2021
View reviewed changes
Copy link
Collaborator

@aabmass aabmass left a comment •
edited

Choose a reason for hiding this comment

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

Nice, looks good! Just add back the GKE container case and we should be good

opentelemetry-exporter-gcp-trace/src/opentelemetry/exporter/cloud_trace/__init__.py Outdated Show resolved Hide resolved
opentelemetry-exporter-gcp-trace/src/opentelemetry/exporter/cloud_trace/__init__.py Outdated Show resolved Hide resolved
opentelemetry-exporter-gcp-trace/tests/test_cloud_trace_exporter.py Show resolved Hide resolved
@dhpollack
Copy link
Contributor Author

With regards to the failing lint on ci. I am not getting this on my local machine. Not sure what is going on there.

aabmass
aabmass approved these changes Jun 2, 2021
View reviewed changes
Copy link
Collaborator

@aabmass aabmass left a comment

Choose a reason for hiding this comment

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

Thanks for working through my comments, LGTM!

@aabmass
Copy link
Collaborator

aabmass commented Jun 2, 2021

/gcbrun

@aabmass
Copy link
Collaborator

aabmass commented Jun 2, 2021

@dhpollack could you rename this PR to something more descriptive of what we landed on, and add an entry to the changelog file? I'll merge after that 😃

@dhpollack dhpollack changed the title Add OTEL_RESOURCE_ATTRIBUTES to exporter Add option for additional resource attributes with regex to trace exporter Jun 2, 2021
@dhpollack
Copy link
Contributor Author

@dhpollack could you rename this PR to something more descriptive of what we landed on, and add an entry to the changelog file? I'll merge after that smiley

@aabmass done! Thanks for all the help.

@aabmass
Copy link
Collaborator

aabmass commented Jun 2, 2021 •
edited

/gcbrun

@aabmass aabmass merged commit 48de0cc into GoogleCloudPlatform:master Jun 2, 2021
@aabmass aabmass mentioned this pull request Aug 6, 2021
Closed
@aabmass aabmass mentioned this pull request Oct 19, 2021
Closed
@aabmass aabmass mentioned this pull request Jan 12, 2022
Closed
@aabmass aabmass mentioned this pull request Jan 24, 2022
Closed
@aabmass aabmass mentioned this pull request Feb 4, 2022
Merged
@CyberHippo CyberHippo mentioned this pull request Mar 7, 2022
Closed
@NivedhaSenthil NivedhaSenthil mentioned this pull request Apr 3, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aabmass aabmass aabmass approved these changes

Assignees
No one assigned
Labels
cla: yes enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@dhpollack @aabmass @jsuereth