From 22d65e210357c446530966179c4e3d399d2bc40b Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Tue, 25 Apr 2023 13:26:22 -0700 Subject: [PATCH 1/3] Removed InstrumentationScope class. --- src/OpenTelemetry.Api/InstrumentationScope.cs | 65 ------------------- src/OpenTelemetry.Api/Logs/Logger.cs | 37 +++++++---- src/OpenTelemetry.Api/Logs/LoggerProvider.cs | 65 ++++++++++++++++--- src/OpenTelemetry.Api/Logs/NoopLogger.cs | 2 +- .../Internal/Shims/IsExternalInit.cs | 0 5 files changed, 84 insertions(+), 85 deletions(-) delete mode 100644 src/OpenTelemetry.Api/InstrumentationScope.cs rename src/{OpenTelemetry.Api => OpenTelemetry}/Internal/Shims/IsExternalInit.cs (100%) diff --git a/src/OpenTelemetry.Api/InstrumentationScope.cs b/src/OpenTelemetry.Api/InstrumentationScope.cs deleted file mode 100644 index 9b6540d032e..00000000000 --- a/src/OpenTelemetry.Api/InstrumentationScope.cs +++ /dev/null @@ -1,65 +0,0 @@ -// -// Copyright The OpenTelemetry Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. -// - -#nullable enable - -namespace OpenTelemetry; - -/// -/// Contains details about the library emitting telemetry. -/// -internal sealed class InstrumentationScope -{ - /// - /// Initializes a new instance of the class. - /// - public InstrumentationScope() - : this(name: null) - { - } - - /// - /// Initializes a new instance of the class. - /// - /// Optional name identifying the instrumentation library. - public InstrumentationScope(string? name) - { - this.Name = string.IsNullOrWhiteSpace(name) - ? string.Empty - : name!; - } - - /// - /// Gets the name identifying the instrumentation library. - /// - public string Name { get; } - - /// - /// Gets the version of the instrumentation library. - /// - public string? Version { get; init; } - - /// - /// Gets the schema url of the instrumentation library. - /// - public string? SchemaUrl { get; init; } - - /// - /// Gets the attributes which should be associated with log records created - /// by the instrumentation library. - /// - public IReadOnlyDictionary? Attributes { get; init; } -} diff --git a/src/OpenTelemetry.Api/Logs/Logger.cs b/src/OpenTelemetry.Api/Logs/Logger.cs index 6addbadd20e..3253f1ee2d0 100644 --- a/src/OpenTelemetry.Api/Logs/Logger.cs +++ b/src/OpenTelemetry.Api/Logs/Logger.cs @@ -16,8 +16,6 @@ #nullable enable -using OpenTelemetry.Internal; - namespace OpenTelemetry.Logs; /// @@ -28,20 +26,29 @@ internal abstract class Logger /// /// Initializes a new instance of the class. /// - /// . - protected Logger(InstrumentationScope instrumentationScope) + /// Optional name identifying the instrumentation library. + protected Logger(string? name) { - Guard.ThrowIfNull(instrumentationScope); - - this.InstrumentationScope = instrumentationScope; + this.Name = string.IsNullOrWhiteSpace(name) + ? string.Empty + : name!; } /// - /// Gets the associated - /// with the logger. + /// Gets the name identifying the instrumentation library. + /// + public string Name { get; } + + /// + /// Gets the version of the instrumentation library. /// - public InstrumentationScope InstrumentationScope { get; } + public string? Version { get; private set; } + + /// + /// Gets the attributes which should be associated with log records created + /// by the instrumentation library. + /// + public IReadOnlyDictionary? Attributes { get; private set; } /// /// Emit a log. @@ -51,4 +58,12 @@ protected Logger(InstrumentationScope instrumentationScope) public abstract void EmitLog( in LogRecordData data, in LogRecordAttributeList attributes = default); + + internal void SetInstrumentationScope( + string? version, + IReadOnlyDictionary? attributes) + { + this.Version = version; + this.Attributes = attributes; + } } diff --git a/src/OpenTelemetry.Api/Logs/LoggerProvider.cs b/src/OpenTelemetry.Api/Logs/LoggerProvider.cs index 6e0f07658b6..cc63b82cddf 100644 --- a/src/OpenTelemetry.Api/Logs/LoggerProvider.cs +++ b/src/OpenTelemetry.Api/Logs/LoggerProvider.cs @@ -16,6 +16,10 @@ #nullable enable +#if NETSTANDARD2_1_OR_GREATER || NET6_0_OR_GREATER +using System.Diagnostics.CodeAnalysis; +#endif + namespace OpenTelemetry.Logs; /// @@ -37,21 +41,66 @@ protected LoggerProvider() /// /// instance. public Logger GetLogger() - => this.GetLogger(new InstrumentationScope()); + => this.GetLogger(name: null, version: null, attributes: null); /// /// Gets a logger with the given name. /// /// Optional name identifying the instrumentation library. /// instance. - public Logger GetLogger(string name) - => this.GetLogger(new InstrumentationScope(name)); + public Logger GetLogger(string? name) + => this.GetLogger(name, version: null, attributes: null); /// - /// Gets a logger with given instrumentation scope. + /// Gets a logger with the given name and version. /// - /// . - /// . - public virtual Logger GetLogger(InstrumentationScope instrumentationScope) - => NoopLogger; + /// Optional name identifying the instrumentation library. + /// Optional version of the instrumentation library. + /// instance. + public Logger GetLogger(string? name, string? version) + => this.GetLogger(name, version, attributes: null); + + /// + /// Gets a logger with the given name, version, and attributes. + /// + /// Optional name identifying the instrumentation + /// library. + /// Optional version of the instrumentation + /// library. + /// Optional attributes which should be associated + /// with log records created by the instrumentation library. + /// instance. + public Logger GetLogger( + string? name, + string? version, + IReadOnlyDictionary? attributes) + { + if (!this.TryCreateLogger(name, out var logger)) + { + return NoopLogger; + } + + logger!.SetInstrumentationScope( + version, + attributes); + + return logger; + } + + /// + /// Try to create a logger with the given name. + /// + /// Optional name identifying the instrumentation library. + /// . + /// if the logger was created. + protected virtual bool TryCreateLogger( + string? name, +#if NETSTANDARD2_1_OR_GREATER || NET6_0_OR_GREATER + [NotNullWhen(true)] +#endif + out Logger? logger) + { + logger = null; + return false; + } } diff --git a/src/OpenTelemetry.Api/Logs/NoopLogger.cs b/src/OpenTelemetry.Api/Logs/NoopLogger.cs index 62c74ae8e6e..3493687647a 100644 --- a/src/OpenTelemetry.Api/Logs/NoopLogger.cs +++ b/src/OpenTelemetry.Api/Logs/NoopLogger.cs @@ -21,7 +21,7 @@ namespace OpenTelemetry.Logs; internal sealed class NoopLogger : Logger { public NoopLogger() - : base(instrumentationScope: new()) + : base(name: null) { } diff --git a/src/OpenTelemetry.Api/Internal/Shims/IsExternalInit.cs b/src/OpenTelemetry/Internal/Shims/IsExternalInit.cs similarity index 100% rename from src/OpenTelemetry.Api/Internal/Shims/IsExternalInit.cs rename to src/OpenTelemetry/Internal/Shims/IsExternalInit.cs From a49971b9fb01dc490c610f4b44d06a6d40cd5f6d Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Tue, 25 Apr 2023 13:47:15 -0700 Subject: [PATCH 2/3] LoggerProvider tests. --- src/OpenTelemetry.Api/Logs/LoggerProvider.cs | 7 -- .../Logs/LogRecordDataTests.cs | 2 + .../Logs/LoggerProviderTests.cs | 112 ++++++++++++++++++ 3 files changed, 114 insertions(+), 7 deletions(-) create mode 100644 test/OpenTelemetry.Api.Tests/Logs/LoggerProviderTests.cs diff --git a/src/OpenTelemetry.Api/Logs/LoggerProvider.cs b/src/OpenTelemetry.Api/Logs/LoggerProvider.cs index cc63b82cddf..5545f907314 100644 --- a/src/OpenTelemetry.Api/Logs/LoggerProvider.cs +++ b/src/OpenTelemetry.Api/Logs/LoggerProvider.cs @@ -29,13 +29,6 @@ internal class LoggerProvider : BaseProvider { private static readonly NoopLogger NoopLogger = new(); - /// - /// Initializes a new instance of the class. - /// - protected LoggerProvider() - { - } - /// /// Gets a logger. /// diff --git a/test/OpenTelemetry.Api.Tests/Logs/LogRecordDataTests.cs b/test/OpenTelemetry.Api.Tests/Logs/LogRecordDataTests.cs index 09d15e2b0f8..700e68b03ef 100644 --- a/test/OpenTelemetry.Api.Tests/Logs/LogRecordDataTests.cs +++ b/test/OpenTelemetry.Api.Tests/Logs/LogRecordDataTests.cs @@ -14,6 +14,8 @@ // limitations under the License. // +#nullable enable + using System.Diagnostics; using Xunit; diff --git a/test/OpenTelemetry.Api.Tests/Logs/LoggerProviderTests.cs b/test/OpenTelemetry.Api.Tests/Logs/LoggerProviderTests.cs new file mode 100644 index 00000000000..250d98a39ea --- /dev/null +++ b/test/OpenTelemetry.Api.Tests/Logs/LoggerProviderTests.cs @@ -0,0 +1,112 @@ +// +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +#nullable enable + +using Xunit; + +namespace OpenTelemetry.Logs.Tests; + +public sealed class LoggerProviderTests +{ + [Fact] + public void NoopLoggerReturnedTest() + { + using var provider = new LoggerProvider(); + + var attributes = new Dictionary() + { + ["key1"] = "value1", + }; + + var logger = provider.GetLogger(name: "TestLogger", version: "Version", attributes); + + Assert.NotNull(logger); + Assert.Equal(typeof(NoopLogger), logger.GetType()); + + Assert.Equal(string.Empty, logger.Name); + Assert.Null(logger.Version); + Assert.Null(logger.Attributes); + } + + [Fact] + public void LoggerReturnedWithInstrumentationScopeTest() + { + using var provider = new TestLoggerProvider(); + + var attributes = new Dictionary() + { + ["key1"] = "value1", + }; + + var logger = provider.GetLogger(name: "TestLogger", version: "Version", attributes); + + Assert.NotNull(logger); + Assert.Equal(typeof(TestLogger), logger.GetType()); + + Assert.Equal("TestLogger", logger.Name); + Assert.Equal("Version", logger.Version); + Assert.Equal(attributes, logger.Attributes); + + logger = provider.GetLogger(name: "TestLogger", version: "Version"); + + Assert.NotNull(logger); + Assert.Equal(typeof(TestLogger), logger.GetType()); + + Assert.Equal("TestLogger", logger.Name); + Assert.Equal("Version", logger.Version); + Assert.Null(logger.Attributes); + + logger = provider.GetLogger(name: "TestLogger"); + + Assert.NotNull(logger); + Assert.Equal(typeof(TestLogger), logger.GetType()); + + Assert.Equal("TestLogger", logger.Name); + Assert.Null(logger.Version); + Assert.Null(logger.Attributes); + + logger = provider.GetLogger(); + + Assert.NotNull(logger); + Assert.Equal(typeof(TestLogger), logger.GetType()); + + Assert.Equal(string.Empty, logger.Name); + Assert.Null(logger.Version); + Assert.Null(logger.Attributes); + } + + private sealed class TestLoggerProvider : LoggerProvider + { + protected override bool TryCreateLogger(string? name, out Logger? logger) + { + logger = new TestLogger(name); + return true; + } + } + + private sealed class TestLogger : Logger + { + public TestLogger(string? name) + : base(name) + { + } + + public override void EmitLog(in LogRecordData data, in LogRecordAttributeList attributes = default) + { + } + } +} From bbb1af7bcdd4ff4b4e3e37e3d4dfb6a24e6342c1 Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Tue, 25 Apr 2023 14:19:26 -0700 Subject: [PATCH 3/3] Code review. --- src/OpenTelemetry.Api/Logs/Logger.cs | 10 +------ src/OpenTelemetry.Api/Logs/LoggerProvider.cs | 24 +++-------------- .../Logs/LoggerProviderTests.cs | 27 ++----------------- 3 files changed, 6 insertions(+), 55 deletions(-) diff --git a/src/OpenTelemetry.Api/Logs/Logger.cs b/src/OpenTelemetry.Api/Logs/Logger.cs index 3253f1ee2d0..acbd19906c3 100644 --- a/src/OpenTelemetry.Api/Logs/Logger.cs +++ b/src/OpenTelemetry.Api/Logs/Logger.cs @@ -44,12 +44,6 @@ protected Logger(string? name) /// public string? Version { get; private set; } - /// - /// Gets the attributes which should be associated with log records created - /// by the instrumentation library. - /// - public IReadOnlyDictionary? Attributes { get; private set; } - /// /// Emit a log. /// @@ -60,10 +54,8 @@ public abstract void EmitLog( in LogRecordAttributeList attributes = default); internal void SetInstrumentationScope( - string? version, - IReadOnlyDictionary? attributes) + string? version) { this.Version = version; - this.Attributes = attributes; } } diff --git a/src/OpenTelemetry.Api/Logs/LoggerProvider.cs b/src/OpenTelemetry.Api/Logs/LoggerProvider.cs index 5545f907314..194f45f4382 100644 --- a/src/OpenTelemetry.Api/Logs/LoggerProvider.cs +++ b/src/OpenTelemetry.Api/Logs/LoggerProvider.cs @@ -34,7 +34,7 @@ internal class LoggerProvider : BaseProvider /// /// instance. public Logger GetLogger() - => this.GetLogger(name: null, version: null, attributes: null); + => this.GetLogger(name: null, version: null); /// /// Gets a logger with the given name. @@ -42,7 +42,7 @@ public Logger GetLogger() /// Optional name identifying the instrumentation library. /// instance. public Logger GetLogger(string? name) - => this.GetLogger(name, version: null, attributes: null); + => this.GetLogger(name, version: null); /// /// Gets a logger with the given name and version. @@ -51,31 +51,13 @@ public Logger GetLogger(string? name) /// Optional version of the instrumentation library. /// instance. public Logger GetLogger(string? name, string? version) - => this.GetLogger(name, version, attributes: null); - - /// - /// Gets a logger with the given name, version, and attributes. - /// - /// Optional name identifying the instrumentation - /// library. - /// Optional version of the instrumentation - /// library. - /// Optional attributes which should be associated - /// with log records created by the instrumentation library. - /// instance. - public Logger GetLogger( - string? name, - string? version, - IReadOnlyDictionary? attributes) { if (!this.TryCreateLogger(name, out var logger)) { return NoopLogger; } - logger!.SetInstrumentationScope( - version, - attributes); + logger!.SetInstrumentationScope(version); return logger; } diff --git a/test/OpenTelemetry.Api.Tests/Logs/LoggerProviderTests.cs b/test/OpenTelemetry.Api.Tests/Logs/LoggerProviderTests.cs index 250d98a39ea..b780ca16fa1 100644 --- a/test/OpenTelemetry.Api.Tests/Logs/LoggerProviderTests.cs +++ b/test/OpenTelemetry.Api.Tests/Logs/LoggerProviderTests.cs @@ -27,19 +27,13 @@ public void NoopLoggerReturnedTest() { using var provider = new LoggerProvider(); - var attributes = new Dictionary() - { - ["key1"] = "value1", - }; - - var logger = provider.GetLogger(name: "TestLogger", version: "Version", attributes); + var logger = provider.GetLogger(name: "TestLogger", version: "Version"); Assert.NotNull(logger); Assert.Equal(typeof(NoopLogger), logger.GetType()); Assert.Equal(string.Empty, logger.Name); Assert.Null(logger.Version); - Assert.Null(logger.Attributes); } [Fact] @@ -47,28 +41,13 @@ public void LoggerReturnedWithInstrumentationScopeTest() { using var provider = new TestLoggerProvider(); - var attributes = new Dictionary() - { - ["key1"] = "value1", - }; - - var logger = provider.GetLogger(name: "TestLogger", version: "Version", attributes); - - Assert.NotNull(logger); - Assert.Equal(typeof(TestLogger), logger.GetType()); - - Assert.Equal("TestLogger", logger.Name); - Assert.Equal("Version", logger.Version); - Assert.Equal(attributes, logger.Attributes); - - logger = provider.GetLogger(name: "TestLogger", version: "Version"); + var logger = provider.GetLogger(name: "TestLogger", version: "Version"); Assert.NotNull(logger); Assert.Equal(typeof(TestLogger), logger.GetType()); Assert.Equal("TestLogger", logger.Name); Assert.Equal("Version", logger.Version); - Assert.Null(logger.Attributes); logger = provider.GetLogger(name: "TestLogger"); @@ -77,7 +56,6 @@ public void LoggerReturnedWithInstrumentationScopeTest() Assert.Equal("TestLogger", logger.Name); Assert.Null(logger.Version); - Assert.Null(logger.Attributes); logger = provider.GetLogger(); @@ -86,7 +64,6 @@ public void LoggerReturnedWithInstrumentationScopeTest() Assert.Equal(string.Empty, logger.Name); Assert.Null(logger.Version); - Assert.Null(logger.Attributes); } private sealed class TestLoggerProvider : LoggerProvider