Skip to content

Commit

Permalink
Merge branch 'main' into 5013-http-known-methods
Browse files Browse the repository at this point in the history
  • Loading branch information
ngruson committed Jan 2, 2024
2 parents 688602f + 0889e8d commit 92c4df1
Show file tree
Hide file tree
Showing 10 changed files with 518 additions and 31 deletions.
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!
4 changes: 4 additions & 0 deletions src/OpenTelemetry.Exporter.OpenTelemetryProtocol/CHANGELOG.md
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.

52 changes: 47 additions & 5 deletions src/OpenTelemetry/Logs/ILogger/OpenTelemetryLoggingExtensions.cs
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
1 change: 0 additions & 1 deletion src/OpenTelemetry/OpenTelemetry.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
</PropertyGroup>

<ItemGroup>
<PackageReference Include="Microsoft.Extensions.Logging" />
<PackageReference Include="Microsoft.Extensions.Logging.Configuration" />
</ItemGroup>

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>());

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);
}
}
}

0 comments on commit 92c4df1

Please sign in to comment.