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

Telemetry provider PoC #5107

Closed
wants to merge 3 commits into from

Conversation

mx-psi
Copy link
Member

@mx-psi mx-psi commented Mar 28, 2022

Description:

This is a proof of concept in support of #4970.

A TelemetryProvider is defined to support overriding the telemetry on the Collector. A default implementation is provided, but alternatives can be provided to override telemetry providers and also to deal with #4971.

The TelemetryProvider interface has two methods: the SetupTelemetry and ZPages. The latter is needed since the span processor needs to be passed as part of the zpages server registration. This could be a separate, optional interface, similar to what happens now for ZPages on component.Host.

Link to tracking Issue: #4970

@codecov
Copy link

codecov bot commented Mar 28, 2022

Codecov Report

Merging #5107 (ac30277) into main (2acc422) will decrease coverage by 0.03%.
The diff coverage is 82.22%.

@@            Coverage Diff             @@
##             main    #5107      +/-   ##
==========================================
- Coverage   90.57%   90.54%   -0.04%     
==========================================
  Files         189      190       +1     
  Lines       11116    11143      +27     
==========================================
+ Hits        10068    10089      +21     
- Misses        826      830       +4     
- Partials      222      224       +2     
Impacted Files Coverage Δ
service/collector.go 73.61% <66.66%> (-2.29%) ⬇️
service/telemetry_provider.go 90.00% <90.00%> (ø)
pdata/internal/common.go 94.25% <0.00%> (-0.77%) ⬇️
exporter/exporterhelper/queued_retry.go 99.27% <0.00%> (+2.91%) ⬆️

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 2acc422...ac30277. Read the comment docs.

@mx-psi mx-psi force-pushed the mx-psi/telemetryprovider branch 2 times, most recently from 5c97e97 to 1101695 Compare March 29, 2022 14:19
@github-actions
Copy link
Contributor

This PR was marked stale due to lack of activity. It will be closed in 14 days.

@github-actions github-actions bot added the Stale label Apr 20, 2022
@mx-psi mx-psi removed the Stale label Apr 20, 2022
@mx-psi
Copy link
Member Author

mx-psi commented Apr 29, 2022

I refactored this a bit to show how to skip GRPC logger as part of a 'test telemetry provider' and to adapt to main changes

@github-actions
Copy link
Contributor

This PR was marked stale due to lack of activity. It will be closed in 14 days.

@github-actions github-actions bot added the Stale label May 14, 2022
@mx-psi mx-psi removed the Stale label May 23, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Jun 7, 2022

This PR was marked stale due to lack of activity. It will be closed in 14 days.

@github-actions github-actions bot added the Stale label Jun 7, 2022
@mx-psi mx-psi removed the Stale label Jun 7, 2022
@github-actions
Copy link
Contributor

This PR was marked stale due to lack of activity. It will be closed in 14 days.

@github-actions
Copy link
Contributor

This PR was marked stale due to lack of activity. It will be closed in 14 days.

@github-actions github-actions bot added the Stale label Jul 12, 2022
@mx-psi mx-psi removed the Stale label Jul 12, 2022
@github-actions
Copy link
Contributor

This PR was marked stale due to lack of activity. It will be closed in 14 days.

@github-actions
Copy link
Contributor

This PR was marked stale due to lack of activity. It will be closed in 14 days.

@github-actions github-actions bot added the Stale label Aug 13, 2022
@github-actions
Copy link
Contributor

Closed as inactive. Feel free to reopen if this PR is still being worked on.

@jmacd
Copy link
Contributor

jmacd commented Jul 27, 2023

FWIW #8111 (comment)
I recommend we lean on the OTel global providers instead of supporting a different way to inject those dependencies.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants