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

[OneCollector] Registration extension + tenant token tweaks #1032

Merged

Conversation

CodeBlanch
Copy link
Member

@CodeBlanch CodeBlanch commented Feb 22, 2023

Changes

  • Tenant token is now parsed automatically from the instrumentation key
  • Registration extensions now operate on a builder instead of options directly. This is kind of a WIP I think it will make things easier as more and more features get added. But it may be removed before stable.

TODOs

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

@CodeBlanch CodeBlanch requested a review from a team February 22, 2023 19:17
@github-actions github-actions bot requested a review from reyang February 22, 2023 19:18
@codecov
Copy link

codecov bot commented Feb 22, 2023

Codecov Report

❗ No coverage uploaded for pull request base (main@e080a57). Click here to learn what that means.
The diff coverage is 55.31%.

❗ Current head b9d7f62 differs from pull request most recent head bd06696. Consider uploading reports for the commit bd06696 to get more accurate results

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #1032   +/-   ##
=======================================
  Coverage        ?   69.44%           
=======================================
  Files           ?      200           
  Lines           ?     7631           
  Branches        ?        0           
=======================================
  Hits            ?     5299           
  Misses          ?     2332           
  Partials        ?        0           
Impacted Files Coverage Δ
...neCollector/Logs/OneCollectorLogExporterOptions.cs 90.90% <ø> (ø)
...eCollector/OneCollectorExporterTransportOptions.cs 72.22% <ø> (ø)
...neCollector/Logs/OneCollectorLogExporterBuilder.cs 31.81% <31.81%> (ø)
...neCollectorOpenTelemetryLoggerOptionsExtensions.cs 76.00% <64.70%> (ø)
...porter.OneCollector/OneCollectorExporterOptions.cs 100.00% <100.00%> (ø)

/// <param name="instrumentationKey">Instrumentation key.</param>
/// <returns>The supplied <see cref="OneCollectorLogExporterBuilder"/> for
/// call chaining.</returns>
public OneCollectorLogExporterBuilder SetInstrumentationKey(string instrumentationKey)
Copy link
Member

Choose a reason for hiding this comment

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

Not blocking this PR - maybe use "ConnectionString" and get rid of these domain specific concepts (as they always change).

Copy link
Member Author

Choose a reason for hiding this comment

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

Opened #1037

@CodeBlanch CodeBlanch merged commit d0190c3 into open-telemetry:main Feb 22, 2023
@CodeBlanch CodeBlanch deleted the onecollector-parse-tenanttoken branch February 22, 2023 20:56
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.

3 participants