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

[onecollector] Remove ext.dt.traceFlags #1345

Merged
merged 3 commits into from
Sep 11, 2023

Conversation

CodeBlanch
Copy link
Member

@CodeBlanch CodeBlanch commented Sep 7, 2023

Fixes #1313

Changes

  • Removes ext.dt.traceFlags because it isn't currently supported by OneCollector

TODOs

  • Appropriate CHANGELOG.md updated for non-trivial changes

@CodeBlanch CodeBlanch added the comp:exporter.onecollector Things related to OpenTelemetry.Exporter.OneCollector label Sep 7, 2023
@CodeBlanch CodeBlanch requested a review from a team as a code owner September 7, 2023 18:21
@github-actions github-actions bot requested a review from reyang September 7, 2023 18:21
@codecov
Copy link

codecov bot commented Sep 7, 2023

Codecov Report

Merging #1345 (4743734) into main (c4784de) will increase coverage by 0.63%.
The diff coverage is n/a.

❗ Current head 4743734 differs from pull request most recent head 305bb93. Consider uploading reports for the commit 305bb93 to get more accurate results

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1345      +/-   ##
==========================================
+ Coverage   73.86%   74.50%   +0.63%     
==========================================
  Files         266      266              
  Lines        9600     9655      +55     
==========================================
+ Hits         7091     7193     +102     
+ Misses       2509     2462      -47     
Files Changed Coverage
...rialization/LogRecordCommonSchemaJsonSerializer.cs ø

Copy link
Member

@reyang reyang left a comment

Choose a reason for hiding this comment

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

@CodeBlanch what's the plan for testing these changes in the future?

@CodeBlanch
Copy link
Member Author

@CodeBlanch what's the plan for testing these changes in the future?

@reyang Good question! I have an integration test I spun up locally to verify this. My plan as a separate work stream is to add that test based on an envvar key (or two) and possibly a workflow which uses a secret (assuming GitHub has such a thing). Taking a bit of care so I don't leak the key/spam the service.

@Kielek Kielek merged commit 630667b into open-telemetry:main Sep 11, 2023
21 of 22 checks passed
@CodeBlanch CodeBlanch deleted the onecollector-traceflags branch September 11, 2023 16:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
comp:exporter.onecollector Things related to OpenTelemetry.Exporter.OneCollector
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Issue with CommonSchemaJsonSerializer [OpenTelemetry.Exporter.OneCollector]
4 participants