Add support for OTEL_SERVICE_NAME environmental variable #2209

pellared · 2021-07-29T12:57:47Z

Partially addresses #1453.

This feature is important so that we can reuse the SDK in https://github.com/open-telemetry/opentelemetry-dotnet-instrumentation.

Changes

ResourceBuilder.AddEnvironmentVariableDetector handles OTEL_SERVICE_NAME
environmental variable.

TODO

Make the env var detectors out-of-the-box Add support for OTEL_SERVICE_NAME environmental variable #2209 (comment)

pellared · 2021-07-29T13:05:18Z

src/OpenTelemetry/Resources/ResourceBuilderExtensions.cs

-        /// Adds resource attributes parsed from an environment variable to a
-        /// <see cref="ResourceBuilder"/> following the <a
+        /// Adds resource attributes parsed from OTEL_RESOURCE_ATTRIBUTES, OTEL_SERVICE_NAME environment variables
+        /// to a <see cref="ResourceBuilder"/> following the <a
        /// href="https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/resource/sdk.md#specifying-resource-information-via-an-environment-variable">Resource
        /// SDK</a>.
        /// </summary>
        /// <param name="resourceBuilder"><see cref="ResourceBuilder"/>.</param>
        /// <returns>Returns <see cref="ResourceBuilder"/> for chaining.</returns>
        public static ResourceBuilder AddEnvironmentVariableDetector(this ResourceBuilder resourceBuilder)


Couldn't this detector be out-of-the-box? This is what other languages do.

For sure something for a separate PR and maybe even needs some GH Issue for discussion.

+1 on making this out-of-the-box

I will make a separate PR to address it

codecov · 2021-07-29T13:30:52Z

Codecov Report

Merging #2209 (48b041f) into main (f644373) will decrease coverage by 0.02%.
The diff coverage is 68.75%.

@@            Coverage Diff             @@
##             main    #2209      +/-   ##
==========================================
- Coverage   80.17%   80.14%   -0.03%     
==========================================
  Files         226      227       +1     
  Lines        7161     7173      +12     
==========================================
+ Hits         5741     5749       +8     
- Misses       1420     1424       +4

Impacted Files	Coverage Δ
...OpenTelemetry/Resources/OtelEnvResourceDetector.cs	`82.35% <33.33%> (ø)`
...lemetry/Resources/OtelServiceNameEnvVarDetector.cs	`75.00% <75.00%> (ø)`
...enTelemetry/Resources/ResourceBuilderExtensions.cs	`88.46% <100.00%> (ø)`
...Zipkin/Implementation/ZipkinExporterEventSource.cs	`63.63% <0.00%> (-9.10%)`	⬇️
...ter.ZPages/Implementation/ZPagesActivityTracker.cs	`97.14% <0.00%> (-2.86%)`	⬇️
...Telemetry/Internal/SelfDiagnosticsEventListener.cs	`97.63% <0.00%> (+0.78%)`	⬆️

reyang · 2021-07-29T15:50:17Z

src/OpenTelemetry/README.md

  * [Instrumentation](#instrumentation)
  * [Processor](#processor)
  * [Resource](#resource)
  * [Sampler](#sampler)
 * [Advanced topics](#advanced-topics)
  * [Propagators](#propagators)
 * [Troubleshooting](#troubleshooting)
+  * [Configuration Parameters](#configuration-parameters)
+  * [Remarks](#remarks)


Both Configuration Parameters and Remarks seem to be too detailed and generic for the ToC. Perhaps we want to remove the H3 (e.g. replace ### Remarks with **Remarks:**).

@open-telemetry/dotnet-maintainers WDYT - should I change it?

BTW I think that the Propagators section should be under Tracing configuration - and not under Advanced topics.

I will make a separate PR for it as I did not get feedback on what is the preference

src/OpenTelemetry/README.md

Co-authored-by: Reiley Yang <reyang@microsoft.com>

pellared · 2021-07-29T16:19:16Z

Flaky test:

Failed OpenTelemetry.Instrumentation.AspNetCore.Tests.MetricTests.RequestMetricIsCaptured [113 ms]
  Error Message:
   System.Exception : Unsupported Type
  Stack Trace:
     at OpenTelemetry.Metrics.SumMetricAggregator.get_Sum() in /home/runner/work/opentelemetry-dotnet/opentelemetry-dotnet/src/OpenTelemetry/Metrics/MetricAggregators/SumMetricAggregator.cs:line 74
   at OpenTelemetry.Instrumentation.AspNetCore.Tests.MetricTests.RequestMetricIsCaptured() in /home/runner/work/opentelemetry-dotnet/opentelemetry-dotnet/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/MetricTests.cs:line 93
--- End of stack trace from previous location ---
fail: Microsoft.AspNetCore.Diagnostics.DeveloperExceptionPageMiddleware[1]
      An unhandled exception has occurred while executing the request.
      System.Exception: exception description
         at OpenTelemetry.Instrumentation.AspNetCore.Tests.IncomingRequestsCollectionsIsAccordingToTheSpecTests.TestCallbackMiddlewareImpl.ProcessAsync(HttpContext context) in /home/runner/work/opentelemetry-dotnet/opentelemetry-dotnet/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/IncomingRequestsCollectionsIsAccordingToTheSpecTests.cs:line 176
         at TestApp.AspNetCore._5._0.CallbackMiddleware.InvokeAsync(HttpContext context) in /home/runner/work/opentelemetry-dotnet/opentelemetry-dotnet/test/TestApp.AspNetCore.5.0/CallbackMiddleware.cs:line 35
         at Microsoft.AspNetCore.Diagnostics.DeveloperExceptionPageMiddleware.Invoke(HttpContext context)
warn: Microsoft.AspNetCore.Diagnostics.DeveloperExceptionPageMiddleware[2]
      The response has already started, the error page middleware will not be executed.
fail: Microsoft.AspNetCore.Diagnostics.DeveloperExceptionPageMiddleware[1]
      An unhandled exception has occurred while executing the request.
      System.Exception: exception description
         at OpenTelemetry.Instrumentation.AspNetCore.Tests.IncomingRequestsCollectionsIsAccordingToTheSpecTests.TestCallbackMiddlewareImpl.ProcessAsync(HttpContext context) in /home/runner/work/opentelemetry-dotnet/opentelemetry-dotnet/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/IncomingRequestsCollectionsIsAccordingToTheSpecTests.cs:line 176
         at TestApp.AspNetCore._5._0.CallbackMiddleware.InvokeAsync(HttpContext context) in /home/runner/work/opentelemetry-dotnet/opentelemetry-dotnet/test/TestApp.AspNetCore.5.0/CallbackMiddleware.cs:line 35
         at Microsoft.AspNetCore.Diagnostics.DeveloperExceptionPageMiddleware.Invoke(HttpContext context)
warn: Microsoft.AspNetCore.Diagnostics.DeveloperExceptionPageMiddleware[2]
      The response has already started, the error page middleware will not be executed.

src/OpenTelemetry/README.md

src/OpenTelemetry/Resources/OtelServiceNameEnvVarDetector.cs

Co-authored-by: Paulo Janotti <pjanotti@splunk.com>

src/OpenTelemetry/Resources/OtelServiceNameEnvVarDetector.cs

src/OpenTelemetry/Resources/OtelEnvResourceDetector.cs

CodeBlanch

LGTM

pellared · 2021-07-30T20:18:16Z

Any idea why EasyCLA is not being triggered?

cijothomas · 2021-07-30T20:18:33Z

Any idea why EasyCLA is not being triggered?

no! same issue with another PR also

linux-foundation-easycla · 2021-07-31T16:22:30Z

The committers are authorized under a signed CLA.

✅ Robert Pająk (c20a073, 756ec0b, b76da07, edca8cd, bec5a63, 960408f, a05f90e, 17c7633, b6989e5, 72038c0, d519865, 3268d71, e0a9aa4)

pellared · 2021-08-02T06:04:30Z

@cijothomas EasyCLA is working

pellared · 2021-08-06T09:15:03Z

@open-telemetry/dotnet-maintainers PTAL

Handle OTEL_SERVICE_NAME environmental variable

c20a073

pellared requested a review from a team July 29, 2021 12:57

pellared added 2 commits July 29, 2021 15:01

Fix lint issues

756ec0b

Refine markdown

b76da07

pellared commented Jul 29, 2021

View reviewed changes

pellared added 3 commits July 29, 2021 15:06

Fix build

edca8cd

dotnet-format

bec5a63

Fix warnings

960408f

reyang reviewed Jul 29, 2021

View reviewed changes

src/OpenTelemetry/README.md Outdated Show resolved Hide resolved

Update src/OpenTelemetry/README.md

a05f90e

Co-authored-by: Reiley Yang <reyang@microsoft.com>

Merge branch 'main' into otel-service-name

17c7633

dszmigielski approved these changes Jul 30, 2021

View reviewed changes

pjanotti reviewed Jul 30, 2021

View reviewed changes

src/OpenTelemetry/README.md Outdated Show resolved Hide resolved

src/OpenTelemetry/README.md Outdated Show resolved Hide resolved

src/OpenTelemetry/Resources/OtelServiceNameEnvVarDetector.cs Outdated Show resolved Hide resolved

pellared and others added 3 commits July 30, 2021 08:32

Update src/OpenTelemetry/README.md

b6989e5

Co-authored-by: Paulo Janotti <pjanotti@splunk.com>

Merge branch 'main' into otel-service-name

72038c0

Update README.md

d519865

pellared requested a review from pjanotti July 30, 2021 06:50

pjanotti approved these changes Jul 30, 2021

View reviewed changes

CodeBlanch reviewed Jul 30, 2021

View reviewed changes

src/OpenTelemetry/Resources/OtelServiceNameEnvVarDetector.cs Outdated Show resolved Hide resolved

CodeBlanch reviewed Jul 30, 2021

View reviewed changes

src/OpenTelemetry/Resources/OtelServiceNameEnvVarDetector.cs Outdated Show resolved Hide resolved

pellared added 2 commits July 30, 2021 20:26

Make resource env vars public

3268d71

OtelServiceNameEnvVarDetector handles SecurityException

e0a9aa4

pellared requested review from CodeBlanch and reyang July 30, 2021 18:33

CodeBlanch reviewed Jul 30, 2021

View reviewed changes

src/OpenTelemetry/Resources/OtelEnvResourceDetector.cs Show resolved Hide resolved

CodeBlanch approved these changes Jul 30, 2021

View reviewed changes

pellared closed this Jul 30, 2021

pellared reopened this Jul 30, 2021

cijothomas mentioned this pull request Jul 31, 2021

EasyCLA check failing open-telemetry/community#795

Closed

pellared closed this Jul 31, 2021

pellared reopened this Jul 31, 2021

pellared closed this Aug 2, 2021

pellared reopened this Aug 2, 2021

pellared force-pushed the otel-service-name branch from 46ac66f to e0a9aa4 Compare August 2, 2021 06:03

Merge branch 'main' into otel-service-name

b65c03c

cijothomas and others added 3 commits August 2, 2021 09:16

Merge branch 'main' into otel-service-name

f36c035

Merge branch 'main' into otel-service-name

a938e86

OtelEnvResourceDetector catches SecurityException

b54fe52

Merge branch 'main' into otel-service-name

48b041f

cijothomas approved these changes Aug 10, 2021

View reviewed changes

cijothomas merged commit 24226a5 into open-telemetry:main Aug 10, 2021

pellared deleted the otel-service-name branch August 10, 2021 18:25

pellared mentioned this pull request Aug 11, 2021

Add environment variable detectors to the default ResourceBuilder #2247

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for OTEL_SERVICE_NAME environmental variable #2209

Add support for OTEL_SERVICE_NAME environmental variable #2209

pellared commented Jul 29, 2021 •

edited

Loading

pellared Jul 29, 2021

reyang Jul 29, 2021

pellared Jul 30, 2021

codecov bot commented Jul 29, 2021 •

edited

Loading

reyang Jul 29, 2021

pellared Jul 30, 2021

pellared Aug 6, 2021

pellared commented Jul 29, 2021

CodeBlanch left a comment

pellared commented Jul 30, 2021

cijothomas commented Jul 30, 2021

linux-foundation-easycla bot commented Jul 31, 2021 •

edited

Loading

pellared commented Aug 2, 2021

pellared commented Aug 6, 2021

Add support for OTEL_SERVICE_NAME environmental variable #2209

Add support for OTEL_SERVICE_NAME environmental variable #2209

Conversation

pellared commented Jul 29, 2021 • edited Loading

Changes

TODO

pellared Jul 29, 2021

Choose a reason for hiding this comment

reyang Jul 29, 2021

Choose a reason for hiding this comment

pellared Jul 30, 2021

Choose a reason for hiding this comment

codecov bot commented Jul 29, 2021 • edited Loading

Codecov Report

reyang Jul 29, 2021

Choose a reason for hiding this comment

pellared Jul 30, 2021

Choose a reason for hiding this comment

pellared Aug 6, 2021

Choose a reason for hiding this comment

pellared commented Jul 29, 2021

CodeBlanch left a comment

Choose a reason for hiding this comment

pellared commented Jul 30, 2021

cijothomas commented Jul 30, 2021

linux-foundation-easycla bot commented Jul 31, 2021 • edited Loading

pellared commented Aug 2, 2021

pellared commented Aug 6, 2021

pellared commented Jul 29, 2021 •

edited

Loading

codecov bot commented Jul 29, 2021 •

edited

Loading

linux-foundation-easycla bot commented Jul 31, 2021 •

edited

Loading