[Logs-branch] More spec-compliant handling of InstrumentationScope (#…

…3762)

* More spec-compliant handling of InstrumentationScope in logging.

* Added throw behavior comment.

* Code review.

src/OpenTelemetry.Api/InstrumentationScope.cs

@@ -25,6 +25,8 @@ namespace OpenTelemetry;
 /// </summary>
 public sealed class InstrumentationScope
 {
+    internal IReadOnlyDictionary<string, object>? AttributeBacking;
+
     /// <summary>
     /// Initializes a new instance of the <see cref="InstrumentationScope"/> class.
     /// </summary>
@@ -63,5 +65,9 @@ public InstrumentationScope(string? name)
     /// Gets the attributes which should be associated with log records created
     /// by the instrumentation library.
     /// </summary>
-    public IReadOnlyDictionary<string, object>? Attributes { get; init; }
+    public IReadOnlyDictionary<string, object>? Attributes
+    {
+        get => this.AttributeBacking;
+        init => this.AttributeBacking = value;
+    }
 }

src/OpenTelemetry.Api/Internal/SemanticConventions.cs

@@ -107,5 +107,8 @@ internal static class SemanticConventions
         public const string AttributeExceptionType = "exception.type";
         public const string AttributeExceptionMessage = "exception.message";
         public const string AttributeExceptionStacktrace = "exception.stacktrace";
+
+        public const string AttributeLogEventDomain = "event.domain";
+        public const string AttributeLogEventName = "event.name";
     }
 }

src/OpenTelemetry.Api/Logs/LoggerOptions.cs

@@ -16,7 +16,9 @@
 
 #nullable enable
 
+using System.Collections.Generic;
 using OpenTelemetry.Internal;
+using OpenTelemetry.Trace;
 
 namespace OpenTelemetry.Logs;
 
@@ -62,7 +64,45 @@ public LoggerOptions(InstrumentationScope instrumentationScope)
     /// <summary>
     /// Gets the domain of events emitted by the instrumentation library.
     /// </summary>
-    public string? EventDomain { get; init; }
+    public string? EventDomain
+    {
+        get
+        {
+            var attributes = this.InstrumentationScope.AttributeBacking;
+            if (attributes != null && attributes.TryGetValue(SemanticConventions.AttributeLogEventDomain, out var eventDomain))
+            {
+                return eventDomain as string;
+            }
+
+            return null;
+        }
+
+        init
+        {
+            var attributes = this.InstrumentationScope.AttributeBacking;
+
+            Dictionary<string, object> newAttributes = new(attributes?.Count + 1 ?? 1);
+
+            if (attributes != null)
+            {
+                foreach (var kvp in attributes)
+                {
+                    newAttributes.Add(kvp.Key, kvp.Value);
+                }
+            }
+
+            if (value != null)
+            {
+                newAttributes[SemanticConventions.AttributeLogEventDomain] = value;
+            }
+            else
+            {
+                newAttributes.Remove(SemanticConventions.AttributeLogEventDomain);
+            }
+
+            this.InstrumentationScope.AttributeBacking = newAttributes;
+        }
+    }
 
     /// <summary>
     /// Gets a value indicating whether or not trace context should

src/OpenTelemetry.Exporter.Console/ConsoleLogRecordExporter.cs

@@ -161,6 +161,29 @@ void ProcessScope(LogRecordScope scope, ConsoleLogRecordExporter exporter)
                     }
                 }
 
+                var instrumentationScope = logRecord.InstrumentationScope;
+                if (instrumentationScope != null)
+                {
+                    this.WriteLine($"{"\nInstrumentationScope associated with LogRecord:",-RightPaddingLength}");
+                    this.WriteLine($"{"Name:",-RightPaddingLength}{logRecord.InstrumentationScope.Name}");
+                    if (instrumentationScope.Version != null)
+                    {
+                        this.WriteLine($"{"Version:",-RightPaddingLength}{logRecord.InstrumentationScope.Version}");
+                    }
+
+                    if (instrumentationScope.Attributes != null)
+                    {
+                        this.WriteLine("Attributes (Key:Value):");
+                        foreach (var attribute in instrumentationScope.Attributes)
+                        {
+                            if (ConsoleTagTransformer.Instance.TryTransformTag(attribute, out var result))
+                            {
+                                this.WriteLine($"{string.Empty,-4}{result}");
+                            }
+                        }
+                    }
+                }
+
                 this.WriteLine(string.Empty);
             }
 

src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/LogRecordExtensions.cs

@@ -33,29 +33,73 @@ internal static class LogRecordExtensions
     {
         private static readonly string[] SeverityTextMapping = new string[]
         {
-            null, "Trace", "Debug", "Information", "Warning", "Error", "Fatal",
+            "Trace", "Debug", "Information", "Warning", "Error", "Fatal",
         };
 
         internal static void AddBatch(
             this OtlpCollector.ExportLogsServiceRequest request,
             OtlpResource.Resource processResource,
             in Batch<LogRecord> logRecordBatch)
         {
+            Dictionary<string, OtlpLogs.ScopeLogs> logsByLibrary = new Dictionary<string, OtlpLogs.ScopeLogs>();
             var resourceLogs = new OtlpLogs.ResourceLogs
             {
                 Resource = processResource,
             };
             request.ResourceLogs.Add(resourceLogs);
 
-            var scopeLogs = new OtlpLogs.ScopeLogs();
-            resourceLogs.ScopeLogs.Add(scopeLogs);
+            OtlpLogs.ScopeLogs currentScopeLogs = null;
 
             foreach (var logRecord in logRecordBatch)
             {
                 var otlpLogRecord = logRecord.ToOtlpLog();
                 if (otlpLogRecord != null)
                 {
-                    scopeLogs.LogRecords.Add(otlpLogRecord);
+                    var instrumentationScope = logRecord.InstrumentationScope;
+
+                    var instrumentationScopeName = instrumentationScope.Name ?? string.Empty;
+
+                    if (currentScopeLogs == null || currentScopeLogs.Scope.Name != instrumentationScopeName)
+                    {
+                        if (!logsByLibrary.TryGetValue(instrumentationScopeName, out var scopeLogs))
+                        {
+                            var scope = new OtlpCommon.InstrumentationScope()
+                            {
+                                Name = instrumentationScopeName,
+                            };
+
+                            if (instrumentationScope?.Version != null)
+                            {
+                                scope.Version = instrumentationScope.Version;
+                            }
+
+                            var attributes = instrumentationScope?.Attributes;
+                            if (attributes != null)
+                            {
+                                foreach (var attribute in attributes)
+                                {
+                                    if (OtlpKeyValueTransformer.Instance.TryTransformTag(
+                                        attribute,
+                                        out var otlpAttribute))
+                                    {
+                                        scope.Attributes.Add(otlpAttribute);
+                                    }
+                                }
+                            }
+
+                            scopeLogs = new OtlpLogs.ScopeLogs
+                            {
+                                Scope = scope,
+                            };
+
+                            logsByLibrary.Add(instrumentationScopeName, scopeLogs);
+                            resourceLogs.ScopeLogs.Add(scopeLogs);
+                        }
+
+                        currentScopeLogs = scopeLogs;
+                    }
+
+                    currentScopeLogs.LogRecords.Add(otlpLogRecord);
                 }
             }
         }
@@ -71,9 +115,13 @@ internal static OtlpLogs.LogRecord ToOtlpLog(this LogRecord logRecord)
                 {
                     TimeUnixNano = (ulong)logRecord.Timestamp.ToUnixTimeNanoseconds(),
                     SeverityNumber = GetSeverityNumber(logRecord.Severity),
-                    SeverityText = SeverityTextMapping[logRecord.Severity.HasValue ? ((int)logRecord.Severity.Value) + 1 : 0],
                 };
 
+                if (logRecord.Severity.HasValue)
+                {
+                    otlpLogRecord.SeverityText = SeverityTextMapping[(int)logRecord.Severity.Value];
+                }
+
                 if (!string.IsNullOrEmpty(logRecord.CategoryName))
                 {
                     // TODO:

src/OpenTelemetry/Logs/LogRecord.cs

@@ -329,6 +329,7 @@ internal LogRecord Copy()
 
             var copy = new LogRecord()
             {
+                InstrumentationScope = this.InstrumentationScope,
                 Data = this.Data,
                 ILoggerData = this.ILoggerData.Copy(),
                 Attributes = this.Attributes == null ? null : new List<KeyValuePair<string, object?>>(this.Attributes),

src/OpenTelemetry/Logs/LoggerSdk.cs

@@ -20,6 +20,7 @@
 using System.Collections.Generic;
 using System.Diagnostics;
 using OpenTelemetry.Internal;
+using OpenTelemetry.Trace;
 
 namespace OpenTelemetry.Logs;
 
@@ -44,9 +45,13 @@ public LoggerSdk(
     /// <inheritdoc />
     public override void EmitEvent(string name, in LogRecordData data, in LogRecordAttributeList attributes = default)
     {
+        // Note: This method will throw if event.name or event.domain is missing
+        // or null. This was done intentionally see discussion:
+        // https://github.com/open-telemetry/opentelemetry-specification/pull/2768#discussion_r972447436
+
         Guard.ThrowIfNullOrWhitespace(name);
 
-        string eventDomain = this.EnsureEventDomain();
+        this.EnsureEventDomain();
 
         var provider = this.loggerProvider;
         var processor = provider.Processor;
@@ -65,8 +70,7 @@ public override void EmitEvent(string name, in LogRecordData data, in LogRecordA
 
             Debug.Assert(exportedAttributes != null, "exportedAttributes was null");
 
-            exportedAttributes!.Add(new KeyValuePair<string, object?>("event.name", name));
-            exportedAttributes!.Add(new KeyValuePair<string, object?>("event.domain", eventDomain));
+            exportedAttributes!.Add(new KeyValuePair<string, object?>(SemanticConventions.AttributeLogEventName, name));
 
             logRecord.Attributes = exportedAttributes;
 
@@ -104,7 +108,7 @@ public override void EmitLog(in LogRecordData data, in LogRecordAttributeList at
         }
     }
 
-    private string EnsureEventDomain()
+    private void EnsureEventDomain()
     {
         string? eventDomain = this.eventDomain;
 
@@ -119,7 +123,5 @@ private string EnsureEventDomain()
 
             this.eventDomain = eventDomain;
         }
-
-        return eventDomain!;
     }
 }

test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpLogExporterTests.cs

@@ -23,6 +23,7 @@
 using OpenTelemetry.Exporter.OpenTelemetryProtocol.Implementation;
 using OpenTelemetry.Internal;
 using OpenTelemetry.Logs;
+using OpenTelemetry.Proto.Collector.Logs.V1;
 using OpenTelemetry.Tests;
 using OpenTelemetry.Trace;
 using Xunit;
@@ -485,5 +486,53 @@ public void CheckToOtlpLogRecordExceptionAttributes()
             Assert.Contains(SemanticConventions.AttributeExceptionStacktrace, otlpLogRecordAttributes);
             Assert.Contains(logRecord.Exception.ToInvariantString(), otlpLogRecordAttributes);
         }
+
+        [Fact]
+        public void CheckAddBatchInstrumentationScopeProcessed()
+        {
+            List<LogRecord> exportedLogRecords = new();
+
+            using (var provider = Sdk.CreateLoggerProviderBuilder()
+                .AddInMemoryExporter(exportedLogRecords)
+                .Build())
+            {
+                var loggerA = provider.GetLogger(new InstrumentationScope("testLogger1")
+                {
+                    Attributes = new Dictionary<string, object> { ["mycustom.key1"] = "value1" },
+                });
+                var loggerB = provider.GetLogger(
+                    new LoggerOptions(
+                        new InstrumentationScope("testLogger2")
+                        {
+                            Attributes = new Dictionary<string, object> { ["mycustom.key2"] = "value2" },
+                        })
+                    {
+                        EventDomain = "testLogger2EventDomain",
+                    });
+
+                loggerA.EmitLog(default, default);
+                loggerB.EmitEvent("event1", default, default);
+            }
+
+            Assert.Equal(2, exportedLogRecords.Count);
+
+            var batch = new Batch<LogRecord>(exportedLogRecords.ToArray(), 2);
+
+            ExportLogsServiceRequest request = new();
+
+            request.AddBatch(new(), in batch);
+
+            Assert.Equal(2, request.ResourceLogs[0].ScopeLogs.Count);
+
+            Assert.Equal("testLogger1", request.ResourceLogs[0].ScopeLogs[0].Scope.Name);
+            Assert.Equal("mycustom.key1", request.ResourceLogs[0].ScopeLogs[0].Scope.Attributes[0].Key);
+            Assert.Equal("value1", request.ResourceLogs[0].ScopeLogs[0].Scope.Attributes[0].Value.StringValue);
+
+            Assert.Equal("testLogger2", request.ResourceLogs[0].ScopeLogs[1].Scope.Name);
+            Assert.Equal("mycustom.key2", request.ResourceLogs[0].ScopeLogs[1].Scope.Attributes[0].Key);
+            Assert.Equal("value2", request.ResourceLogs[0].ScopeLogs[1].Scope.Attributes[0].Value.StringValue);
+            Assert.Equal(SemanticConventions.AttributeLogEventDomain, request.ResourceLogs[0].ScopeLogs[1].Scope.Attributes[1].Key);
+            Assert.Equal("testLogger2EventDomain", request.ResourceLogs[0].ScopeLogs[1].Scope.Attributes[1].Value.StringValue);
+        }
     }
 }