Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Do not allow changes to OTLP exporter configuration after instantiation #3066

Merged

Conversation

alanwest
Copy link
Member

Prior to this PR, the underlying HTTP and gRPC export clients kept a handle on the original OtlpExporterOptions instance. This allowed for the options to change thereby allowing the behavior of an OTLP exporter instance to be altered after it's been instantiated.

@alanwest alanwest requested a review from a team March 18, 2022 00:22
@codecov
Copy link

codecov bot commented Mar 18, 2022

Codecov Report

Merging #3066 (301b8d9) into main (ea5d11d) will increase coverage by 0.01%.
The diff coverage is 60.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #3066      +/-   ##
==========================================
+ Coverage   84.70%   84.71%   +0.01%     
==========================================
  Files         259      259              
  Lines        9119     9121       +2     
==========================================
+ Hits         7724     7727       +3     
+ Misses       1395     1394       -1     
Impacted Files Coverage Δ
...ementation/ExportClient/OtlpGrpcLogExportClient.cs 0.00% <0.00%> (ø)
...tation/ExportClient/OtlpGrpcMetricsExportClient.cs 35.71% <0.00%> (ø)
...entation/ExportClient/OtlpGrpcTraceExportClient.cs 50.00% <50.00%> (ø)
...mentation/ExportClient/BaseOtlpGrpcExportClient.cs 61.11% <75.00%> (-1.39%) ⬇️
...mentation/ExportClient/BaseOtlpHttpExportClient.cs 100.00% <100.00%> (ø)
...tation/ExportClient/OtlpHttpMetricsExportClient.cs 66.66% <100.00%> (ø)
...entation/ExportClient/OtlpHttpTraceExportClient.cs 90.47% <100.00%> (ø)
...ter.ZPages/Implementation/ZPagesActivityTracker.cs 100.00% <0.00%> (+2.85%) ⬆️
...ZPages/Implementation/ZPagesExporterEventSource.cs 62.50% <0.00%> (+6.25%) ⬆️

#if NETSTANDARD2_1 || NET5_0_OR_GREATER
internal GrpcChannel Channel { get; set; }
#else
internal Channel Channel { get; set; }
#endif

internal Uri Endpoint { get; }
Copy link
Member

Choose a reason for hiding this comment

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

minor: internal readonly?

Copy link
Member

@reyang reyang Mar 18, 2022

Choose a reason for hiding this comment

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

never mind, I see that it only has "get"

@cijothomas cijothomas merged commit 45964ca into open-telemetry:main Mar 18, 2022
@alanwest alanwest deleted the alanwest/otlp-immutable-options branch September 30, 2022 22:05
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants