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

feat(exporter): Jaeger and Zipkin exporter use logr as the logging interface #3500

Merged
merged 8 commits into from Dec 8, 2022
Merged

feat(exporter): Jaeger and Zipkin exporter use logr as the logging interface #3500

merged 8 commits into from Dec 8, 2022

Conversation

aniaan
Copy link
Contributor

@aniaan aniaan commented Nov 30, 2022

close: #3497

Please help to see if this change is appropriate, cc @MadVikingGod

@aniaan
feat(exporter): Jaeger and Zipkin exporter use `github.com/go-logr/lo…
eb27827 
…gr` as the logging interface, and add the WithLogr option
@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Nov 30, 2022 •
edited

CLA Signed

The committers listed above are authorized under a signed CLA.

  • ✅ login: aniaan / name: aniaan (eb27827)

@codecov
Copy link

codecov bot commented Nov 30, 2022 •
edited

Codecov Report

Merging #3500 (b3aa39a) into main (8644a79) will decrease coverage by 0.0%.
The diff coverage is 55.5%.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##            main   #3500     +/-   ##
=======================================
- Coverage   77.9%   77.9%   -0.1%     
=======================================
  Files        164     164             
  Lines      11699   11703      +4     
=======================================
+ Hits        9119    9121      +2     
- Misses      2385    2387      +2     
  Partials     195     195
Impacted Files Coverage Δ
exporters/jaeger/agent.go 72.1% <ø> (ø)
exporters/jaeger/uploader.go 47.5% <0.0%> (-0.7%) ⬇️
exporters/jaeger/reconnecting_udp_client.go 89.9% <33.3%> (ø)
exporters/zipkin/zipkin.go 67.3% <100.0%> (+0.6%) ⬆️

MadVikingGod
MadVikingGod approved these changes Nov 30, 2022
View reviewed changes
Copy link
Contributor

@MadVikingGod MadVikingGod left a comment

Choose a reason for hiding this comment

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

LGTM,

exporters/zipkin/zipkin.go Outdated Show resolved Hide resolved
exporters/jaeger/uploader.go Show resolved Hide resolved
@aniaan aniaan requested a review from Aneurysm9 December 2, 2022 06:26
MrAlias
MrAlias reviewed Dec 2, 2022
View reviewed changes
CHANGELOG.md Outdated Show resolved Hide resolved
exporters/zipkin/zipkin.go Outdated Show resolved Hide resolved
@aniaan aniaan requested review from MrAlias and removed request for Aneurysm9 December 2, 2022 15:50
@MrAlias MrAlias merged commit 4e76347 into open-telemetry:main Dec 8, 2022
@MrAlias MrAlias added this to the Release v1.12.0 milestone Jan 27, 2023
@MrAlias MrAlias mentioned this pull request Jan 28, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Aneurysm9 Aneurysm9 Aneurysm9 left review comments

@dmathieu dmathieu dmathieu approved these changes

@MadVikingGod MadVikingGod MadVikingGod approved these changes

@MrAlias MrAlias MrAlias approved these changes

@jmacd jmacd Awaiting requested review from jmacd

@evantorrie evantorrie Awaiting requested review from evantorrie evantorrie is a code owner

@XSAM XSAM Awaiting requested review from XSAM XSAM is a code owner

@dashpole dashpole Awaiting requested review from dashpole dashpole is a code owner

@pellared pellared Awaiting requested review from pellared pellared is a code owner

@hanyuancheung hanyuancheung Awaiting requested review from hanyuancheung hanyuancheung is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
Release v1.12.0
Development

Successfully merging this pull request may close these issues.

Exporter: WithLogger uses the github.com/go-logr/logr interface to replace the standard library logger
5 participants
@aniaan @dmathieu @Aneurysm9 @MadVikingGod @MrAlias