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

Telemetry provider PoC #5107

Closed
wants to merge 3 commits into from
Closed

Telemetry provider PoC #5107

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

@mx-psi mx-psi mentioned this pull request Mar 28, 2022
Open
@codecov
Copy link

codecov bot commented Mar 28, 2022 •
edited
Loading

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
@mx-psi mx-psi force-pushed the mx-psi/telemetryprovider branch from 1101695 to f42add2 Compare April 5, 2022 09:25
@mx-psi mx-psi mentioned this pull request Apr 6, 2022
Closed
This was referenced Apr 11, 2022
Closed
Closed
@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 mx-psi force-pushed the mx-psi/telemetryprovider branch from 615d81c to ac30277 Compare April 29, 2022 10:28
@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 github-actions bot added the Stale label Jun 22, 2022
@bogdandrutu bogdandrutu removed the Stale label Jun 27, 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 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 github-actions bot added the Stale label Jul 27, 2022
@bogdandrutu bogdandrutu removed the Stale label Jul 29, 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 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.

@github-actions github-actions bot closed this Aug 27, 2022
@mx-psi mx-psi mentioned this pull request Jul 26, 2023
Closed
@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
Reviewers
No reviews
Assignees
No one assigned
Labels
Stale
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@mx-psi @jmacd @bogdandrutu