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

Support customized OTLP endpoint for log example #4066

Merged
merged 6 commits into from
Jan 10, 2023

Conversation

ThomsonTan
Copy link
Contributor

Changes

The current OTLP example doesn't accept endpoint from command line. This PR add the support following the convention from OTLP trace/metrics exporter.

For significant contributions please make sure you have completed the following items:

  • Appropriate CHANGELOG.md updated for non-trivial changes
  • Design discussion issue #
  • Changes in public API reviewed

@ThomsonTan ThomsonTan requested a review from a team January 9, 2023 18:06
Comment on lines +79 to +82
if (!string.IsNullOrWhiteSpace(options.Endpoint))
{
otlpOptions.Endpoint = new Uri(options.Endpoint);
}
Copy link
Member

Choose a reason for hiding this comment

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

Hmm, I guess TestMetrics.cs is also missing this functionality. At a minimum, probably makes sense to add it there too. Though, seems like there may be an opportunity to share some code between these examples, but maybe makes sense to consider that separately.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added the same Endpoint command line option to metrics example. I did some minor cleanup to remove dup code, but agree that further cleanup could be done separately.

@codecov
Copy link

codecov bot commented Jan 9, 2023

Codecov Report

Merging #4066 (cef25c3) into main (b091200) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #4066   +/-   ##
=======================================
  Coverage   85.61%   85.62%           
=======================================
  Files         289      289           
  Lines       11255    11255           
=======================================
+ Hits         9636     9637    +1     
+ Misses       1619     1618    -1     
Impacted Files Coverage Δ
src/OpenTelemetry/Metrics/MetricStreamIdentity.cs 92.30% <100.00%> (ø)
...enTelemetry/Metrics/StringArrayEqualityComparer.cs 84.21% <100.00%> (ø)
src/OpenTelemetry/Metrics/Tags.cs 86.66% <100.00%> (ø)
src/OpenTelemetry/Trace/SamplingResult.cs 100.00% <100.00%> (ø)
...Listener/Internal/PrometheusExporterEventSource.cs 66.66% <0.00%> (+5.55%) ⬆️

Copy link
Member

@alanwest alanwest left a comment

Choose a reason for hiding this comment

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

LGTM

@alanwest alanwest merged commit f98f8fe into open-telemetry:main Jan 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants