opentelemetry-erlang icon indicating copy to clipboard operation
opentelemetry-erlang copied to clipboard

Add service.instance.id resource default value

Open fcevado opened this issue 2 years ago • 5 comments

based on #402 discussion.

fcevado avatar Jun 25 '22 23:06 fcevado

CLA Signed

The committers listed above are authorized under a signed CLA.

  • :white_check_mark: login: fcevado / name: cevado (abc993419e0fa26458dff6264fc8a92a04565e95, 2b948c16c84f52845e62db370d097254421983d7, 06aaaa0e38adb06525577493454753d932fcdd57, adf2a5a5b825e097c3e8f8462f47f9096869307d)

Codecov Report

Merging #409 (5951364) into main (2a700f8) will decrease coverage by 0.25%. The diff coverage is 50.00%.

@@            Coverage Diff             @@
##             main     #409      +/-   ##
==========================================
- Coverage   73.76%   73.51%   -0.26%     
==========================================
  Files          53       53              
  Lines        1681     1699      +18     
==========================================
+ Hits         1240     1249       +9     
- Misses        441      450       +9     
Flag Coverage Δ
api 68.99% <ø> (ø)
elixir 18.83% <ø> (ø)
erlang 74.95% <50.00%> (-0.28%) :arrow_down:
exporter 73.59% <ø> (ø)
sdk 78.33% <50.00%> (-0.63%) :arrow_down:
zipkin 53.84% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
apps/opentelemetry/src/otel_resource_detector.erl 84.61% <50.00%> (-9.91%) :arrow_down:
apps/opentelemetry/src/otel_resource.erl 77.41% <0.00%> (+3.22%) :arrow_up:

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

codecov[bot] avatar Jun 26 '22 14:06 codecov[bot]

The initial thread also had mentions of maybe generating an ID every time the host name is @localhost since it does not provide a lot of information. Any views on this?

ferd avatar Jun 27 '22 13:06 ferd

i totally forgot about that, my bad. I'll add a commit that verifies that.

fcevado avatar Jun 27 '22 13:06 fcevado

@ferd I guess everything is covered now. :thinking:

fcevado avatar Jun 28 '22 00:06 fcevado

The tests don't actually run, but I pulled this down locally and ran them and they work. So I'm going to merge this and then open a PR to make the tests run.

tsloughter avatar Aug 17 '22 20:08 tsloughter

Sorry for the long delay!

tsloughter avatar Aug 17 '22 20:08 tsloughter