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

[net/http] enhance otelhttp example to support metric #2610

Merged

Conversation

fatsheep9146
Copy link
Contributor

Signed-off-by: Ziqi Zhao zhaoziqi9146@gmail.com

The current otelhttp example only contains the logic of traces, I added the support of metric.

@dmathieu
Copy link
Member

The metrics SDK is currently under heavy revamping to be moved out of beta (see the new_sdk branch).
I think this kind of change should wait for that work to be finished.


}
global.SetMeterProvider(controller)

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change

invalid blank line should be removed.

log.Printf("Error shutting down meter provider: %v", err)
}
}()

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change

Ditto.

@fatsheep9146
Copy link
Contributor Author

@dmathieu @MrAlias Already update to latest metric sdk, please help review this again, thanks!

CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
instrumentation/net/http/otelhttp/example/server/server.go Outdated Show resolved Hide resolved
instrumentation/net/http/otelhttp/example/server/server.go Outdated Show resolved Hide resolved
instrumentation/net/http/otelhttp/example/server/server.go Outdated Show resolved Hide resolved
@fatsheep9146
Copy link
Contributor Author

All comments are resolved @MrAlias

@codecov
Copy link

codecov bot commented Nov 8, 2022

Codecov Report

Merging #2610 (73f5f3c) into main (e97d789) will increase coverage by 0.0%.
The diff coverage is 89.4%.

Additional details and impacted files

Impacted file tree graph

@@          Coverage Diff          @@
##            main   #2610   +/-   ##
=====================================
  Coverage   69.6%   69.6%           
=====================================
  Files        147     147           
  Lines       6785    6795   +10     
=====================================
+ Hits        4725    4735   +10     
  Misses      1944    1944           
  Partials     116     116           
Impacted Files Coverage Δ
samplers/jaegerremote/sampler_remote.go 87.4% <50.0%> (ø)
instrumentation/net/http/otelhttp/handler.go 81.9% <100.0%> (+0.5%) ⬆️
samplers/jaegerremote/sampler_remote_options.go 100.0% <100.0%> (ø)

Signed-off-by: Ziqi Zhao <zhaoziqi9146@gmail.com>
@fatsheep9146
Copy link
Contributor Author

the failed check seems to be flaky, could you trigger this again? @dmathieu @MrAlias

@MrAlias MrAlias merged commit 44d8196 into open-telemetry:main Nov 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants