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

feat: add optional filter for OTel resource attributes #361

Merged

Conversation

valerio
Copy link
Contributor

@valerio valerio commented Dec 15, 2021

Fixes #347

This PR adds support for exporting OpenTelemetry resource attributes that match a regular expression, as was suggested in the linked issue.

@aabmass aabmass self-assigned this Dec 20, 2021
@jonbcampos-alto
Copy link

very excited to see this go through!

@codecov
Copy link

codecov bot commented Jan 10, 2022

Codecov Report

Merging #361 (887eb6a) into main (28ee4ad) will increase coverage by 0.07%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #361      +/-   ##
==========================================
+ Coverage   95.48%   95.56%   +0.07%     
==========================================
  Files          12       12              
  Lines         421      428       +7     
  Branches       80       81       +1     
==========================================
+ Hits          402      409       +7     
  Misses         19       19              
Impacted Files Coverage Δ
...es/opentelemetry-cloud-trace-exporter/src/trace.ts 95.16% <100.00%> (+0.16%) ⬆️
...pentelemetry-cloud-trace-exporter/src/transform.ts 98.14% <100.00%> (+0.08%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 28ee4ad...887eb6a. Read the comment docs.

@aabmass
Copy link
Contributor

aabmass commented Jan 12, 2022

/gcbrun

Copy link
Contributor

@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.

Thank you for the PR and apologies for not looking sooner. LGTM, just a few nits

Co-authored-by: Aaron Abbott <aaronabbott@google.com>
@valerio
Copy link
Contributor Author

valerio commented Jan 12, 2022

Thanks for the review @aabmass!
I've applied your suggestions, let me know if anything else needs to be done for merging/releasing.

@aabmass aabmass enabled auto-merge (squash) January 12, 2022 22:12
@aabmass
Copy link
Contributor

aabmass commented Jan 12, 2022

I fixed the lint so it should be good to go once the tests pass. Thanks for your contribution!

@aabmass aabmass merged commit 61ef589 into GoogleCloudPlatform:main Jan 12, 2022
@jonbcampos-alto
Copy link

👍 great job everyone! very excited to work this into my projects

@jonbcampos-alto
Copy link

@aabmass question. when does the team usually cut a change to the package? I can clone the repo and get the changes, but curious when there is an official cut.

@aabmass
Copy link
Contributor

aabmass commented Jan 13, 2022

We don't have a regular cadence, but I'll try to cut a release sooner than later. Depends on how long some of the other updates take.

@aabmass
Copy link
Contributor

aabmass commented Jan 21, 2022

This is released in 1.1.0

@jonbcampos-alto
Copy link

@aabmass thank you! the minute you closed the PR I started working it into our microservices and I'm showing it to the engineering team in about an hour. This makes our work with tracing so much easier to use.

@valerio valerio deleted the feat/add-resource-attribute-filter branch March 9, 2022 18:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Trace Exporter drops service.name resource attribute
3 participants