Enable zipkin endpoint configuration via environment variable #1924
Conversation
Codecov Report
@@ Coverage Diff @@
## main #1924 +/- ##
==========================================
+ Coverage 74.63% 79.10% +4.46%
==========================================
Files 217 217
Lines 6974 6987 +13
==========================================
+ Hits 5205 5527 +322
+ Misses 1769 1460 -309
|
…ariable' of https://github.com/vishweshbankwar/opentelemetry-dotnet into vibankwa/Enable-Zipkin-Endpoint-Configuration-via-Env-Variable
…n-via-Env-Variable
…n-via-Env-Variable
…n-via-Env-Variable
…n-via-Env-Variable
test/OpenTelemetry.Exporter.Zipkin.Tests/ZipkinExporterTests.cs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
@cijothomas @vishweshbankwar this is something that we would like to have for auto-instrumentation. I see that it is not filling up the default in case of error parsing env var Uri (and related test) any other thing holding this change?
src/OpenTelemetry.Exporter.Zipkin/Implementation/ZipkinExporterEventSource.cs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
LGTM besides a nit for the README - @cijothomas now that 1.1.0 has shipped can we also merge this one?
|
@cijothomas Anything still blocking this one? |
…n-via-Env-Variable
| { | ||
| this.Endpoint = new Uri(Environment.GetEnvironmentVariable(ZipkinEndpointEnvVar) ?? DefaultZipkinEndpoint); | ||
| } | ||
| catch (Exception ex) |
There was a problem hiding this comment.
minor (may follow up in separate PR):
We can do similar to what Jaeger/OTLP does - catch SecurityException for reading env variable. Handle the Uri parse exception separately. Both should be logged as separate EventSource events.
This avoids catching the generic Exception.
There was a problem hiding this comment.
Sure - will create a separate PR for that.
Fixes #.
#1453
Changes
Please provide a brief description of the changes here.
This PR partially covers #1453
Spec Details
For significant contributions please make sure you have completed the following items:
CHANGELOG.mdupdated for non-trivial changes