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

AddInstrumentation and AddDiagnosticSourceInstrumentation APIs. #1454

Conversation

cijothomas
Copy link
Member

Steps 1,2,3 from #1437 (comment)

Changes

Please provide a brief description of the changes here.

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

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

@cijothomas cijothomas requested a review from a team November 4, 2020 09:00
@codecov
Copy link

codecov bot commented Nov 4, 2020

Codecov Report

Merging #1454 into master will increase coverage by 0.08%.
The diff coverage is 96.15%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1454      +/-   ##
==========================================
+ Coverage   81.95%   82.03%   +0.08%     
==========================================
  Files         230      230              
  Lines        6163     6180      +17     
==========================================
+ Hits         5051     5070      +19     
+ Misses       1112     1110       -2     
Impacted Files Coverage Δ
src/OpenTelemetry/Trace/ActivitySourceAdapter.cs 96.87% <ø> (ø)
src/OpenTelemetry/Trace/TracerProviderBuilder.cs 86.53% <92.85%> (+1.92%) ⬆️
...entation.AspNet/TracerProviderBuilderExtensions.cs 100.00% <100.00%> (ø)
...tion.AspNetCore/TracerProviderBuilderExtensions.cs 100.00% <100.00%> (ø)
...n.GrpcNetClient/TracerProviderBuilderExtensions.cs 100.00% <100.00%> (ø)
...umentation.Http/TracerProviderBuilderExtensions.cs 100.00% <100.00%> (ø)
...ation.SqlClient/TracerProviderBuilderExtensions.cs 100.00% <100.00%> (ø)
...ckExchangeRedis/TracerProviderBuilderExtensions.cs 100.00% <100.00%> (ø)
src/OpenTelemetry/Trace/TracerProviderSdk.cs 95.16% <100.00%> (+0.16%) ⬆️
...emetry.Api/Internal/OpenTelemetryApiEventSource.cs 82.35% <0.00%> (+5.88%) ⬆️
... and 1 more

@cijothomas cijothomas closed this Nov 4, 2020
@cijothomas cijothomas reopened this Nov 4, 2020
@@ -17,5 +17,9 @@
using System.Runtime.CompilerServices;

[assembly: InternalsVisibleTo("OpenTelemetry.Tests" + AssemblyInfo.PublicKey)]
[assembly: InternalsVisibleTo("OpenTelemetry.Instrumentation.Http" + AssemblyInfo.PublicKey)]
Copy link
Member

Choose a reason for hiding this comment

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

nit: sort alphabetically.

internal readonly struct InstrumentationFactory
/// <summary>
/// Adds a DiagnosticSource based instrumentation.
/// This is required for libraries which already is instrumented with
Copy link
Member

@reyang reyang Nov 4, 2020

Choose a reason for hiding this comment

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

Suggested change
/// This is required for libraries which already is instrumented with
/// This is required for libraries which are already instrumented with

Copy link
Member

@reyang reyang left a comment

Choose a reason for hiding this comment

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

LGTM, left couple minor suggestions.

@cijothomas cijothomas merged commit 55cac80 into open-telemetry:master Nov 5, 2020
@cijothomas cijothomas deleted the cijothomas/dsinstrumentationinternal branch November 5, 2020 01: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