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

[sdk] Make ResourceBuilder.AddDetector factory pattern public (attempt 2) #4261

Merged
merged 8 commits into from
Mar 6, 2023
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
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 @@ -23,3 +23,4 @@ static OpenTelemetry.Metrics.MeterProviderBuilderExtensions.SetExemplarFilter(th
~override OpenTelemetry.Metrics.AlwaysOnExemplarFilter.ShouldSample(long value, System.ReadOnlySpan<System.Collections.Generic.KeyValuePair<string, object>> tags) -> bool
~override OpenTelemetry.Metrics.TraceBasedExemplarFilter.ShouldSample(double value, System.ReadOnlySpan<System.Collections.Generic.KeyValuePair<string, object>> tags) -> bool
~override OpenTelemetry.Metrics.TraceBasedExemplarFilter.ShouldSample(long value, System.ReadOnlySpan<System.Collections.Generic.KeyValuePair<string, object>> tags) -> bool
OpenTelemetry.Resources.ResourceBuilder.AddDetector(System.Func<System.IServiceProvider!, OpenTelemetry.Resources.IResourceDetector!>! resourceDetectorFactory) -> OpenTelemetry.Resources.ResourceBuilder!
Original file line number Diff line number Diff line change
Expand Up @@ -23,3 +23,4 @@ static OpenTelemetry.Metrics.MeterProviderBuilderExtensions.SetExemplarFilter(th
~override OpenTelemetry.Metrics.AlwaysOnExemplarFilter.ShouldSample(long value, System.ReadOnlySpan<System.Collections.Generic.KeyValuePair<string, object>> tags) -> bool
~override OpenTelemetry.Metrics.TraceBasedExemplarFilter.ShouldSample(double value, System.ReadOnlySpan<System.Collections.Generic.KeyValuePair<string, object>> tags) -> bool
~override OpenTelemetry.Metrics.TraceBasedExemplarFilter.ShouldSample(long value, System.ReadOnlySpan<System.Collections.Generic.KeyValuePair<string, object>> tags) -> bool
OpenTelemetry.Resources.ResourceBuilder.AddDetector(System.Func<System.IServiceProvider!, OpenTelemetry.Resources.IResourceDetector!>! resourceDetectorFactory) -> OpenTelemetry.Resources.ResourceBuilder!
Original file line number Diff line number Diff line change
Expand Up @@ -23,3 +23,4 @@ static OpenTelemetry.Metrics.MeterProviderBuilderExtensions.SetExemplarFilter(th
~override OpenTelemetry.Metrics.AlwaysOnExemplarFilter.ShouldSample(long value, System.ReadOnlySpan<System.Collections.Generic.KeyValuePair<string, object>> tags) -> bool
~override OpenTelemetry.Metrics.TraceBasedExemplarFilter.ShouldSample(double value, System.ReadOnlySpan<System.Collections.Generic.KeyValuePair<string, object>> tags) -> bool
~override OpenTelemetry.Metrics.TraceBasedExemplarFilter.ShouldSample(long value, System.ReadOnlySpan<System.Collections.Generic.KeyValuePair<string, object>> tags) -> bool
OpenTelemetry.Resources.ResourceBuilder.AddDetector(System.Func<System.IServiceProvider!, OpenTelemetry.Resources.IResourceDetector!>! resourceDetectorFactory) -> OpenTelemetry.Resources.ResourceBuilder!
Original file line number Diff line number Diff line change
Expand Up @@ -23,3 +23,4 @@ static OpenTelemetry.Metrics.MeterProviderBuilderExtensions.SetExemplarFilter(th
~override OpenTelemetry.Metrics.AlwaysOnExemplarFilter.ShouldSample(long value, System.ReadOnlySpan<System.Collections.Generic.KeyValuePair<string, object>> tags) -> bool
~override OpenTelemetry.Metrics.TraceBasedExemplarFilter.ShouldSample(double value, System.ReadOnlySpan<System.Collections.Generic.KeyValuePair<string, object>> tags) -> bool
~override OpenTelemetry.Metrics.TraceBasedExemplarFilter.ShouldSample(long value, System.ReadOnlySpan<System.Collections.Generic.KeyValuePair<string, object>> tags) -> bool
OpenTelemetry.Resources.ResourceBuilder.AddDetector(System.Func<System.IServiceProvider!, OpenTelemetry.Resources.IResourceDetector!>! resourceDetectorFactory) -> OpenTelemetry.Resources.ResourceBuilder!
3 changes: 3 additions & 0 deletions src/OpenTelemetry/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@
* Added Exemplar support.
([#4217](https://github.com/open-telemetry/opentelemetry-dotnet/pull/4217))

* Added `AddDetector` factory overload on `ResourceBuilder`.
([#4261](https://github.com/open-telemetry/opentelemetry-dotnet/pull/4261))

## 1.4.0

Released 2023-Feb-24
Expand Down
4 changes: 4 additions & 0 deletions src/OpenTelemetry/Logs/OpenTelemetryLoggerProvider.cs
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,8 @@ internal OpenTelemetryLoggerProvider(OpenTelemetryLoggerOptions options, IServic

Guard.ThrowIfNull(options);

this.ServiceProvider = serviceProvider;

this.IncludeScopes = options.IncludeScopes;
this.IncludeFormattedMessage = options.IncludeFormattedMessage;
this.ParseStateValues = options.ParseStateValues;
Expand All @@ -96,6 +98,8 @@ internal OpenTelemetryLoggerProvider(OpenTelemetryLoggerOptions options, IServic
OpenTelemetrySdkEventSource.Log.OpenTelemetryLoggerProviderEvent("OpenTelemetryLoggerProvider built successfully.");
}

internal IServiceProvider? ServiceProvider { get; }

internal IExternalScopeProvider? ScopeProvider { get; private set; }

internal ILogRecordPool LogRecordPool => this.threadStaticPool ?? LogRecordSharedPool.Current;
Expand Down
4 changes: 4 additions & 0 deletions src/OpenTelemetry/ProviderExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,10 @@ public static Resource GetDefaultResource(this BaseProvider baseProvider)
{
return meterProviderSdk.ServiceProvider;
}
else if (baseProvider is OpenTelemetryLoggerProvider openTelemetryLoggerProvider)
{
return openTelemetryLoggerProvider.ServiceProvider;
}

return null;
}
Expand Down
26 changes: 16 additions & 10 deletions src/OpenTelemetry/Resources/ResourceBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -132,18 +132,24 @@ public ResourceBuilder AddDetector(IResourceDetector resourceDetector)
/// <summary>
/// Add a <see cref="IResourceDetector"/> to the builder which will be resolved using the application <see cref="IServiceProvider"/>.
/// </summary>
/// <remarks>
/// Note: The supplied <paramref name="resourceDetectorFactory"/> may be
/// called with a <see langword="null"/> <see cref="IServiceProvider"/>
/// for detached <see cref="ResourceBuilder"/> instances. Factories
/// should either throw if a <see langword="null"/> cannot be handled,
/// or return a default <see cref="IResourceDetector"/> when <see
/// cref="IServiceProvider"/> is not available.
/// </remarks>
/// <param name="resourceDetectorFactory">Resource detector factory.</param>
/// <returns>Supplied <see cref="ResourceBuilder"/> for call chaining.</returns>
// Note: This API may be made public if there is a need for it.
internal ResourceBuilder AddDetector(Func<IServiceProvider?, IResourceDetector> resourceDetectorFactory)
public ResourceBuilder AddDetector(Func<IServiceProvider, IResourceDetector> resourceDetectorFactory)
{
Guard.ThrowIfNull(resourceDetectorFactory);

return this.AddDetectorInternal(sp =>
{
if (sp == null)
{
throw new NotSupportedException("IResourceDetector factory pattern is not supported when calling ResourceBuilder.Build() directly.");
}

return resourceDetectorFactory(sp);
});
}

internal ResourceBuilder AddDetectorInternal(Func<IServiceProvider?, IResourceDetector> resourceDetectorFactory)
{
Guard.ThrowIfNull(resourceDetectorFactory);

Expand Down
4 changes: 2 additions & 2 deletions src/OpenTelemetry/Resources/ResourceBuilderExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -122,8 +122,8 @@ public static ResourceBuilder AddEnvironmentVariableDetector(this ResourceBuilde
Lazy<IConfiguration> configuration = new Lazy<IConfiguration>(() => new ConfigurationBuilder().AddEnvironmentVariables().Build());

return resourceBuilder
.AddDetector(sp => new OtelEnvResourceDetector(sp?.GetService<IConfiguration>() ?? configuration.Value))
.AddDetector(sp => new OtelServiceNameEnvVarDetector(sp?.GetService<IConfiguration>() ?? configuration.Value));
.AddDetectorInternal(sp => new OtelEnvResourceDetector(sp?.GetService<IConfiguration>() ?? configuration.Value))
.AddDetectorInternal(sp => new OtelServiceNameEnvVarDetector(sp?.GetService<IConfiguration>() ?? configuration.Value));
}

private static string GetFileVersion()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ public void SetAndConfigureResourceTest()

Assert.Empty(builder.ResourceDetectors);

builder.AddDetector(sp =>
builder.AddDetectorInternal(sp =>
{
serviceProviderTestExecuted = true;
Assert.NotNull(sp);
Expand Down
32 changes: 29 additions & 3 deletions test/OpenTelemetry.Tests/Resources/ResourceTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -498,13 +498,39 @@ public void GetResource_WithServiceNameSetWithTwoEnvVarsAndCode()
}

[Fact]
public void ResourceBuilder_ServiceProvider_Available()
public void ResourceBuilder_AddDetector_Test()
{
bool factoryExecuted = false;

var builder = ResourceBuilder.CreateDefault();

builder.AddDetector(sp =>
{
factoryExecuted = true;
return new NoopResourceDetector();
});

Assert.Throws<NotSupportedException>(() => builder.Build());
Assert.False(factoryExecuted);

var serviceCollection = new ServiceCollection();
using var serviceProvider = serviceCollection.BuildServiceProvider();

builder.ServiceProvider = serviceProvider;

var resource = builder.Build();

Assert.True(factoryExecuted);
}

[Fact]
public void ResourceBuilder_AddDetectorInternal_Test()
{
var builder = ResourceBuilder.CreateDefault();

bool nullTestRun = false;

builder.AddDetector(sp =>
builder.AddDetectorInternal(sp =>
{
nullTestRun = true;
Assert.Null(sp);
Expand All @@ -524,7 +550,7 @@ public void ResourceBuilder_ServiceProvider_Available()

builder.ServiceProvider = serviceProvider;

builder.AddDetector(sp =>
builder.AddDetectorInternal(sp =>
{
validTestRun = true;
Assert.NotNull(sp);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ public void SetAndConfigureResourceTest()

Assert.Empty(builder.ResourceDetectors);

builder.AddDetector(sp =>
builder.AddDetectorInternal(sp =>
{
serviceProviderTestExecuted = true;
Assert.NotNull(sp);
Expand Down