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

Instrument batch processor with Otel Go #6423

Conversation

paivagustavo
Copy link
Member

Description:

  • Instrument batch processor with OpenTelemetry Go.
  • Also fixed a small typo on the bucket definition of the batch_send_size_bytes view of OpenCensus.

Link to tracking Issue: part of #816

Testing:

  • Updated existing tests that validate metrics using the OpenCensus view, to test instrumentation made by both OpenCensus and OpenTelemetry SDK.
  • Locally build and run the collector with the featuregate enabled/disabled.

Note: There are some "whitespace" changes, so I suggest reviewers to hide those when reviewing, it was done to make existing tests to run one time for each SDK.

@paivagustavo paivagustavo requested review from a team and jpkrohling October 27, 2022 19:45
@paivagustavo paivagustavo force-pushed the gustavo/instrument_batch_processor_with_otel_go branch from 124433e to b440529 Compare October 27, 2022 19:46
@codecov
Copy link

codecov bot commented Oct 27, 2022

Codecov Report

Base: 91.12% // Head: 91.00% // Decreases project coverage by -0.11% ⚠️

Coverage data is based on head (659f4ad) compared to base (65dfc32).
Patch coverage: 80.68% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6423      +/-   ##
==========================================
- Coverage   91.12%   91.00%   -0.12%     
==========================================
  Files         241      241              
  Lines       13887    14005     +118     
==========================================
+ Hits        12654    12745      +91     
- Misses        993     1011      +18     
- Partials      240      249       +9     
Impacted Files Coverage Δ
service/telemetry.go 86.44% <66.66%> (-1.14%) ⬇️
processor/batchprocessor/metrics.go 84.71% <79.13%> (-15.29%) ⬇️
processor/batchprocessor/batch_processor.go 90.47% <94.44%> (-0.20%) ⬇️
processor/batchprocessor/factory.go 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

processor/batchprocessor/factory.go Show resolved Hide resolved
processor/batchprocessor/metrics_test.go Outdated Show resolved Hide resolved
processor/batchprocessor/metrics.go Show resolved Hide resolved
processor/batchprocessor/metrics.go Show resolved Hide resolved
@paivagustavo paivagustavo force-pushed the gustavo/instrument_batch_processor_with_otel_go branch from 6cb465c to 3c29b8e Compare November 3, 2022 19:32
@paivagustavo paivagustavo force-pushed the gustavo/instrument_batch_processor_with_otel_go branch 2 times, most recently from c73536f to 79ba658 Compare November 9, 2022 14:46
@paivagustavo paivagustavo requested review from codeboten and removed request for jpkrohling November 9, 2022 16:08
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.

Thanks for addressing my comments @paivagustavo, please take a look at @jmacd's suggestion and resolve conflicts

@paivagustavo paivagustavo force-pushed the gustavo/instrument_batch_processor_with_otel_go branch from 79ba658 to 8aca1eb Compare November 16, 2022 12:49
@codeboten codeboten merged commit 2da6865 into open-telemetry:main Nov 16, 2022
@codeboten codeboten deleted the gustavo/instrument_batch_processor_with_otel_go branch November 16, 2022 23:08
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.

4 participants