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

Add otlp log extension methods for LoggerProviderBuilder #5103

Merged
merged 42 commits into from
Dec 23, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
42 commits
Select commit Hold shift + click to select a range
f206708
draft
Yun-Ting Nov 30, 2023
aa42126
merge main
Yun-Ting Nov 30, 2023
b0c97f8
Merge branch 'main' into yunl/OtlpLoggingExtension
Yun-Ting Dec 1, 2023
e11d4bb
add OTLP exporter to the LoggerProvider
Yun-Ting Dec 1, 2023
36e8560
api doc
Yun-Ting Dec 1, 2023
6723a04
extension APIs and tests
Yun-Ting Dec 1, 2023
3653fd3
more tests
Yun-Ting Dec 2, 2023
64f8df9
EXPOSE_EXPERIMENTAL_FEATURES
Yun-Ting Dec 2, 2023
b15421a
Merge branch 'main' into yunl/OtlpLoggingExtension
Yun-Ting Dec 2, 2023
4255964
public api files
Yun-Ting Dec 2, 2023
0b853e7
Merge branch 'main' into yunl/OtlpLoggingExtension
Yun-Ting Dec 2, 2023
3e2aff0
fix macro
Yun-Ting Dec 8, 2023
ac70c78
Merge branch 'main' into yunl/OtlpLoggingExtension
Yun-Ting Dec 8, 2023
06893fe
Merge branch 'main' into yunl/OtlpLoggingExtension
Yun-Ting Dec 8, 2023
7f28ee5
fixed public api file
Yun-Ting Dec 9, 2023
5fe64d3
Merge branch 'main' into yunl/OtlpLoggingExtension
Yun-Ting Dec 9, 2023
c828fcc
api/test
Yun-Ting Dec 9, 2023
6c4d67b
flag for namedoptions test
Yun-Ting Dec 9, 2023
7276894
test flag
Yun-Ting Dec 11, 2023
7a7e962
mark apis internal when !EXPOSE_EXPERIMENTAL_FEATURES
Yun-Ting Dec 11, 2023
df0adcb
Merge branch 'main' into yunl/OtlpLoggingExtension
Yun-Ting Dec 11, 2023
6cf59f0
test
Yun-Ting Dec 11, 2023
f08622d
Merge branch 'yunl/OtlpLoggingExtension' of https://github.com/Yun-Ti…
Yun-Ting Dec 11, 2023
35406ae
Revert "test"
Yun-Ting Dec 11, 2023
1f99014
internal visible to for project and test
Yun-Ting Dec 11, 2023
ac0494b
Merge branch 'main' into yunl/OtlpLoggingExtension
Yun-Ting Dec 11, 2023
a350e30
Merge branch 'main' into yunl/OtlpLoggingExtension
Yun-Ting Dec 12, 2023
fa4d229
Add ExperimentalAttribute decorations and clean up a few nits.
CodeBlanch Dec 12, 2023
04a036c
Tweaks and fixes.
CodeBlanch Dec 13, 2023
d09f1b0
changelog
Yun-Ting Dec 13, 2023
fa2857a
Merge branch 'main' into yunl/OtlpLoggingExtension
Yun-Ting Dec 13, 2023
604ccf6
Merge branch 'main' into yunl/OtlpLoggingExtension
Yun-Ting Dec 14, 2023
c19868d
Code review.
CodeBlanch Dec 18, 2023
e208fbc
Code review.
CodeBlanch Dec 19, 2023
c4e795f
Clean up.
CodeBlanch Dec 19, 2023
2f51df2
Code review and test improvements.
CodeBlanch Dec 19, 2023
e17f4e7
Nits
CodeBlanch Dec 19, 2023
66ea5e3
Test improvements.
CodeBlanch Dec 19, 2023
8f15ca0
Code review.
CodeBlanch Dec 23, 2023
11d7f4c
Code review.
CodeBlanch Dec 23, 2023
df362e4
Code review.
CodeBlanch Dec 23, 2023
a332701
Merge branch 'main' into yunl/OtlpLoggingExtension
Yun-Ting Dec 23, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@

#if !EXPOSE_EXPERIMENTAL_FEATURES
[assembly: InternalsVisibleTo("OpenTelemetry.Exporter.Console" + AssemblyInfo.PublicKey)]
[assembly: InternalsVisibleTo("OpenTelemetry.Exporter.OpenTelemetryProtocol" + AssemblyInfo.PublicKey)]
[assembly: InternalsVisibleTo("OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests" + AssemblyInfo.PublicKey)]
[assembly: InternalsVisibleTo("OpenTelemetry.Extensions.Hosting" + AssemblyInfo.PublicKey)]
#endif

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
static OpenTelemetry.Logs.OtlpLogExporterHelperExtensions.AddOtlpExporter(this OpenTelemetry.Logs.LoggerProviderBuilder! builder) -> OpenTelemetry.Logs.LoggerProviderBuilder!
static OpenTelemetry.Logs.OtlpLogExporterHelperExtensions.AddOtlpExporter(this OpenTelemetry.Logs.LoggerProviderBuilder! builder, string? name, System.Action<OpenTelemetry.Exporter.OtlpExporterOptions!, OpenTelemetry.Logs.LogRecordExportProcessorOptions!>? configureExporterAndProcessor) -> OpenTelemetry.Logs.LoggerProviderBuilder!
static OpenTelemetry.Logs.OtlpLogExporterHelperExtensions.AddOtlpExporter(this OpenTelemetry.Logs.LoggerProviderBuilder! builder, string? name, System.Action<OpenTelemetry.Exporter.OtlpExporterOptions!>? configureExporter) -> OpenTelemetry.Logs.LoggerProviderBuilder!
static OpenTelemetry.Logs.OtlpLogExporterHelperExtensions.AddOtlpExporter(this OpenTelemetry.Logs.LoggerProviderBuilder! builder, System.Action<OpenTelemetry.Exporter.OtlpExporterOptions!, OpenTelemetry.Logs.LogRecordExportProcessorOptions!>! configureExporterAndProcessor) -> OpenTelemetry.Logs.LoggerProviderBuilder!
static OpenTelemetry.Logs.OtlpLogExporterHelperExtensions.AddOtlpExporter(this OpenTelemetry.Logs.LoggerProviderBuilder! builder, System.Action<OpenTelemetry.Exporter.OtlpExporterOptions!>! configureExporter) -> OpenTelemetry.Logs.LoggerProviderBuilder!
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

## Unreleased

* **Experimental (pre-release builds only):** Added
`LoggerProviderBuilder.AddOtlpExporter` registration extensions.
[#5103](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5103)

## 1.7.0

Released 2023-Dec-08
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,11 @@
<!-- Note: When '$(ExposeExperimentalFeatures)' == 'false' these links are
NOT required because this project sees API + SDK internals -->
<Compile Include="$(RepoRoot)\src\Shared\Guard.cs" Link="Includes\Guard.cs" RequiresExposedExperimentalFeatures="true" />
<Compile Include="$(RepoRoot)\src\Shared\ResourceSemanticConventions.cs" Link="Includes\ResourceSemanticConventions.cs" RequiresExposedExperimentalFeatures="true" />
<Compile Include="$(RepoRoot)\src\Shared\DiagnosticDefinitions.cs" Link="Includes\DiagnosticDefinitions.cs" RequiresExposedExperimentalFeatures="true" />
<Compile Include="$(RepoRoot)\src\Shared\EnvironmentVariables\*.cs" Link="Includes\EnvironmentVariables\%(Filename).cs" RequiresExposedExperimentalFeatures="true" />
<Compile Include="$(RepoRoot)\src\Shared\ExceptionExtensions.cs" Link="Includes\ExceptionExtensions.cs" RequiresExposedExperimentalFeatures="true" />
<Compile Include="$(RepoRoot)\src\Shared\Options\*.cs" Link="Includes\Options\%(Filename).cs" RequiresExposedExperimentalFeatures="true" />
<Compile Include="$(RepoRoot)\src\Shared\ResourceSemanticConventions.cs" Link="Includes\ResourceSemanticConventions.cs" RequiresExposedExperimentalFeatures="true" />
<Compile Include="$(RepoRoot)\src\Shared\SemanticConventions.cs" Link="Includes\SemanticConventions.cs" RequiresExposedExperimentalFeatures="true" />
<Compile Include="$(RepoRoot)\src\Shared\SpanAttributeConstants.cs" Link="Includes\SpanAttributeConstants.cs" RequiresExposedExperimentalFeatures="true" />
<Compile Include="$(RepoRoot)\src\Shared\StatusHelper.cs" Link="Includes\StatusHelper.cs" RequiresExposedExperimentalFeatures="true" />
Expand Down

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#if EXPOSE_EXPERIMENTAL_FEATURES
using System.ComponentModel;
#endif
using System.Diagnostics;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.DependencyInjection.Extensions;
using Microsoft.Extensions.Logging;
Expand Down Expand Up @@ -169,9 +170,15 @@ private static ILoggingBuilder AddOpenTelemetryInternal(

var services = builder.Services;

// Note: This will bind logger options element (eg "Logging:OpenTelemetry") to OpenTelemetryLoggerOptions
// Note: This will bind logger options element (e.g., "Logging:OpenTelemetry") to OpenTelemetryLoggerOptions
RegisterLoggerProviderOptions(services);

/* Note: This ensures IConfiguration is available when using
* IServiceCollections NOT attached to a host. For example when
* performing:
*
* new ServiceCollection().AddLogging(b => b.AddOpenTelemetry())
*/
services.AddOpenTelemetrySharedProviderBuilderServices();

if (configureOptions != null)
Expand Down Expand Up @@ -206,10 +213,45 @@ private static ILoggingBuilder AddOpenTelemetryInternal(

services.TryAddEnumerable(
ServiceDescriptor.Singleton<ILoggerProvider, OpenTelemetryLoggerProvider>(
sp => new OpenTelemetryLoggerProvider(
sp.GetRequiredService<LoggerProvider>(),
sp.GetRequiredService<IOptionsMonitor<OpenTelemetryLoggerOptions>>().CurrentValue,
disposeProvider: false)));
sp =>
{
var state = sp.GetRequiredService<LoggerProviderBuilderSdk>();

var provider = state.Provider;
if (provider == null)
{
/*
* Note:
*
* There is a possibility of a circular reference when
* accessing LoggerProvider from the IServiceProvider.
*
* If LoggerProvider is the first thing accessed, and it
* requires some service which accesses ILogger (for
* example, IHttpClientFactory), then the
* OpenTelemetryLoggerProvider will try to access a new
* (second) LoggerProvider while still in the process of
* building the first one:
*
* LoggerProvider -> IHttpClientFactory ->
* ILoggerFactory -> OpenTelemetryLoggerProvider ->
* LoggerProvider
*
* This check uses the provider reference captured on
* LoggerProviderBuilderSdk during construction of
* LoggerProviderSdk to detect if a provider has already
* been created to give to OpenTelemetryLoggerProvider
* and stop the loop.
*/
provider = sp.GetRequiredService<LoggerProvider>();
Debug.Assert(provider == state.Provider, "state.Provider did not match resolved LoggerProvider.");
}

return new OpenTelemetryLoggerProvider(
provider,
sp.GetRequiredService<IOptionsMonitor<OpenTelemetryLoggerOptions>>().CurrentValue,
disposeProvider: false);
}));

return builder;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,8 @@ public void LogExportResultIsSuccess(OtlpExportProtocol protocol, string endpoin
sp,
exporterOptions,
processorOptions,
new SdkLimitOptions(),
new ExperimentalOptions(),
configureExporterInstance: otlpExporter =>
{
delegatingExporter = new DelegatingExporter<LogRecord>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,125 @@ public class OtlpLogExporterTests : Http2UnencryptedSupportTests
{
private static readonly SdkLimitOptions DefaultSdkLimitOptions = new();

[Fact]
public void AddOtlpExporterWithNamedOptions()
{
int defaultConfigureExporterOptionsInvocations = 0;
int namedConfigureExporterOptionsInvocations = 0;

int defaultConfigureSdkLimitsOptionsInvocations = 0;
int namedConfigureSdkLimitsOptionsInvocations = 0;

using var loggerProvider = Sdk.CreateLoggerProviderBuilder()
.ConfigureServices(services =>
{
services.Configure<OtlpExporterOptions>(o => defaultConfigureExporterOptionsInvocations++);
services.Configure<LogRecordExportProcessorOptions>(o => defaultConfigureExporterOptionsInvocations++);
services.Configure<ExperimentalOptions>(o => defaultConfigureExporterOptionsInvocations++);

services.Configure<OtlpExporterOptions>("Exporter2", o => namedConfigureExporterOptionsInvocations++);
services.Configure<LogRecordExportProcessorOptions>("Exporter2", o => namedConfigureExporterOptionsInvocations++);
services.Configure<ExperimentalOptions>("Exporter2", o => namedConfigureExporterOptionsInvocations++);

services.Configure<OtlpExporterOptions>("Exporter3", o => namedConfigureExporterOptionsInvocations++);
services.Configure<LogRecordExportProcessorOptions>("Exporter3", o => namedConfigureExporterOptionsInvocations++);
services.Configure<ExperimentalOptions>("Exporter3", o => namedConfigureExporterOptionsInvocations++);

services.Configure<SdkLimitOptions>(o => defaultConfigureSdkLimitsOptionsInvocations++);
services.Configure<SdkLimitOptions>("Exporter2", o => namedConfigureSdkLimitsOptionsInvocations++);
services.Configure<SdkLimitOptions>("Exporter3", o => namedConfigureSdkLimitsOptionsInvocations++);
})
.AddOtlpExporter()
.AddOtlpExporter("Exporter2", o => { })
.AddOtlpExporter("Exporter3", o => { })
.Build();

Assert.Equal(3, defaultConfigureExporterOptionsInvocations);
Assert.Equal(6, namedConfigureExporterOptionsInvocations);

// Note: SdkLimitOptions does NOT support named options. We only allow a
// single instance for a given IServiceCollection.
Assert.Equal(1, defaultConfigureSdkLimitsOptionsInvocations);
Assert.Equal(0, namedConfigureSdkLimitsOptionsInvocations);
}

[Fact]
public void UserHttpFactoryCalledWhenUsingHttpProtobuf()
{
OtlpExporterOptions options = new OtlpExporterOptions();

var defaultFactory = options.HttpClientFactory;

int invocations = 0;
options.Protocol = OtlpExportProtocol.HttpProtobuf;
options.HttpClientFactory = () =>
{
invocations++;
return defaultFactory();
};

using (var exporter = new OtlpLogExporter(options))
{
Assert.Equal(1, invocations);
}

using (var provider = Sdk.CreateLoggerProviderBuilder()
.AddOtlpExporter(o =>
{
o.Protocol = OtlpExportProtocol.HttpProtobuf;
o.HttpClientFactory = options.HttpClientFactory;
})
.Build())
{
Assert.Equal(2, invocations);
}

options.HttpClientFactory = null;
Assert.Throws<InvalidOperationException>(() =>
{
using var exporter = new OtlpLogExporter(options);
});
}

[Fact]
public void AddOtlpExporterSetsDefaultBatchExportProcessor()
{
if (Environment.Version.Major == 3)
{
// Adding the OtlpExporter creates a GrpcChannel.
// This switch must be set before creating a GrpcChannel when calling an insecure HTTP/2 endpoint.
// See: https://docs.microsoft.com/aspnet/core/grpc/troubleshoot#call-insecure-grpc-services-with-net-core-client
AppContext.SetSwitch("System.Net.Http.SocketsHttpHandler.Http2UnencryptedSupport", true);
}

var loggerProvider = Sdk.CreateLoggerProviderBuilder()
.AddOtlpExporter()
.Build();

CheckProcessorDefaults();

loggerProvider.Dispose();

void CheckProcessorDefaults()
{
var bindingFlags = BindingFlags.Public | BindingFlags.Instance | BindingFlags.NonPublic;

var processor = typeof(BaseProcessor<LogRecord>)
.Assembly
.GetType("OpenTelemetry.Logs.LoggerProviderSdk")
.GetProperty("Processor", bindingFlags)
.GetValue(loggerProvider) as BatchExportProcessor<LogRecord>;

Assert.NotNull(processor);

var scheduledDelayMilliseconds = typeof(BatchExportProcessor<LogRecord>)
.GetField("scheduledDelayMilliseconds", bindingFlags)
.GetValue(processor);

Assert.Equal(5000, scheduledDelayMilliseconds);
}
}

[Fact]
public void AddOtlpLogExporterReceivesAttributesWithParseStateValueSetToFalse()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,60 @@ public void VerifyExceptionIsThrownWhenImplementationFactoryIsNull()
Assert.Throws<ArgumentNullException>(() => sp.GetRequiredService<LoggerProvider>() as LoggerProviderSdk);
}

[Theory]
[InlineData(true)]
[InlineData(false)]
public void CircularReferenceTest(bool requestLoggerProviderDirectly)
{
var services = new ServiceCollection();

services.AddLogging(logging => logging.AddOpenTelemetry());

services.ConfigureOpenTelemetryLoggerProvider(builder => builder.AddProcessor<TestLogProcessorWithILoggerFactoryDependency>());
utpilla marked this conversation as resolved.
Show resolved Hide resolved

using var sp = services.BuildServiceProvider();

if (requestLoggerProviderDirectly)
{
var provider = sp.GetRequiredService<LoggerProvider>();
Assert.NotNull(provider);
}
else
{
var factory = sp.GetRequiredService<ILoggerFactory>();
Assert.NotNull(factory);
}

var loggerProvider = sp.GetRequiredService<LoggerProvider>() as LoggerProviderSdk;

Assert.NotNull(loggerProvider);

Assert.True(loggerProvider.Processor is TestLogProcessorWithILoggerFactoryDependency);
}

private class TestLogProcessor : BaseProcessor<LogRecord>
{
}

private class TestLogProcessorWithILoggerFactoryDependency : BaseProcessor<LogRecord>
{
private readonly ILogger logger;

public TestLogProcessorWithILoggerFactoryDependency(ILoggerFactory loggerFactory)
{
// Note: It is NOT recommended to log from inside a processor. This
// test is meant to mirror someone injecting IHttpClientFactory
// (which itself uses ILoggerFactory) as part of an exporter. That
// is a more realistic scenario but needs a dependency to do that so
// here we approximate the graph.
this.logger = loggerFactory.CreateLogger("MyLogger");
}

protected override void Dispose(bool disposing)
{
this.logger.LogInformation("Dispose called");

base.Dispose(disposing);
}
}
}