Skip to content

OTLP Exporter: Conditionally compile experimental log features #4762

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.

Sign up for GitHub

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

Closed
alanwest wants to merge 7 commits into from
Closed

OTLP Exporter: Conditionally compile experimental log features #4762

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
5c3188c
Conditionally compile experimental log features
alanwest Aug 11, 2023
96a280b
Stylecops
alanwest Aug 11, 2023
0a5669d
Stylecops
alanwest Aug 11, 2023
2dea303
Stylecops
alanwest Aug 11, 2023
5b00c21
Merge remote-tracking branch 'upstream/main' into alanwest/ilogger-in…
alanwest Aug 11, 2023
6dfcefb
Document experimental features
alanwest Aug 11, 2023
10374f4
Update readme
alanwest Aug 11, 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
8 changes: 6 additions & 2 deletions src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/LogRecordExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,13 @@

using System.Runtime.CompilerServices;
using Google.Protobuf;
#if EXPOSE_EXPERIMENTAL_FEATURES
using OpenTelemetry.Internal;
#endif
using OpenTelemetry.Logs;
#if EXPOSE_EXPERIMENTAL_FEATURES
using OpenTelemetry.Trace;
#endif
using OtlpCollector = OpenTelemetry.Proto.Collector.Logs.V1;
using OtlpCommon = OpenTelemetry.Proto.Common.V1;
using OtlpLogs = OpenTelemetry.Proto.Logs.V1;
Expand Down Expand Up @@ -79,7 +83,7 @@ internal static OtlpLogs.LogRecord ToOtlpLog(this LogRecord logRecord, SdkLimitO

var attributeValueLengthLimit = sdkLimitOptions.AttributeValueLengthLimit;
var attributeCountLimit = sdkLimitOptions.AttributeCountLimit ?? int.MaxValue;

#if EXPOSE_EXPERIMENTAL_FEATURES
Copy link
Member

@CodeBlanch CodeBlanch Aug 11, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we still going to rename the EventId keys? I think we should.

Copy link
Member

@CodeBlanch CodeBlanch Aug 11, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PS: Probably worth mentioning this in CHANGELOG.

Copy link
Member Author

@alanwest alanwest Aug 11, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we still going to rename the EventId keys? I think we should.

See conversation here #4762 (comment)

PS: Probably worth mentioning this in CHANGELOG.

Yes, please hold on merging this. I haven't submitted a changelog entry yet because I'm working on a method to document our experimental features somewhat centrally.

Copy link
Member Author

@alanwest alanwest Aug 11, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@CodeBlanch @utpilla @vishweshbankwar @Kielek Take another look at this PR.

Besides updating the changelog, I reorganized it a bit to propose a slightly different pattern documenting experimental features not only in the changelog but also in the README. This way the documentation of our experimental features doesn't get buried in old changelog entries.

If this pattern looks good, I'm happy to follow up in a separate PR fixing up the documentation for the other components we have introduced experimental features for.

// First add the generic attributes like Category, EventId and Exception,
// so they are less likely being dropped because of AttributeCountLimit.

Expand Down Expand Up @@ -109,7 +113,7 @@ internal static OtlpLogs.LogRecord ToOtlpLog(this LogRecord logRecord, SdkLimitO
otlpLogRecord.AddStringAttribute(SemanticConventions.AttributeExceptionMessage, logRecord.Exception.Message, attributeValueLengthLimit, attributeCountLimit);
otlpLogRecord.AddStringAttribute(SemanticConventions.AttributeExceptionStacktrace, logRecord.Exception.ToInvariantString(), attributeValueLengthLimit, attributeCountLimit);
}

#endif
bool bodyPopulatedFromFormattedMessage = false;
if (logRecord.FormattedMessage != null)
{
Expand Down
101 changes: 57 additions & 44 deletions test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpLogExporterTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,9 @@
using Moq;
using OpenTelemetry.Exporter.OpenTelemetryProtocol.Implementation;
using OpenTelemetry.Exporter.OpenTelemetryProtocol.Implementation.ExportClient;
#if EXPOSE_EXPERIMENTAL_FEATURES
using OpenTelemetry.Internal;
#endif
using OpenTelemetry.Logs;
using OpenTelemetry.Tests;
using OpenTelemetry.Trace;
Expand Down Expand Up @@ -184,25 +186,32 @@ public void OtlpLogRecordTestWhenStateValuesArePopulated()

Assert.NotNull(otlpLogRecord);
Assert.Equal("Hello from tomato 2.99.", otlpLogRecord.Body.StringValue);
#if EXPOSE_EXPERIMENTAL_FEATURES
Assert.Equal(4, otlpLogRecord.Attributes.Count);

var attribute = otlpLogRecord.Attributes[0];
#else
Assert.Equal(3, otlpLogRecord.Attributes.Count);
#endif
var index = 0;
var attribute = otlpLogRecord.Attributes[index];
#if EXPOSE_EXPERIMENTAL_FEATURES
Assert.Equal("dotnet.ilogger.category", attribute.Key);
Assert.Equal("OtlpLogExporterTests", attribute.Value.StringValue);

attribute = otlpLogRecord.Attributes[1];
attribute = otlpLogRecord.Attributes[++index];
#endif
Assert.Equal("name", attribute.Key);
Assert.Equal("tomato", attribute.Value.StringValue);

attribute = otlpLogRecord.Attributes[2];
attribute = otlpLogRecord.Attributes[++index];
Assert.Equal("price", attribute.Key);
Assert.Equal(2.99, attribute.Value.DoubleValue);

attribute = otlpLogRecord.Attributes[3];
attribute = otlpLogRecord.Attributes[++index];
Assert.Equal("{OriginalFormat}", attribute.Key);
Assert.Equal("Hello from {name} {price}.", attribute.Value.StringValue);
}

#if EXPOSE_EXPERIMENTAL_FEATURES
[Fact]
public void CheckToOtlpLogRecordLoggerCategory()
{
Expand Down Expand Up @@ -287,6 +296,7 @@ public void CheckToOtlpLogRecordEventId()
Assert.Contains("Name", otlpLogRecordAttributes);
Assert.Contains("MyEvent10", otlpLogRecordAttributes);
}
#endif

[Fact]
public void CheckToOtlpLogRecordTimestamps()
Expand Down Expand Up @@ -485,6 +495,7 @@ public void CheckToOtlpLogRecordBodyIsPopulated(bool includeFormattedMessage)
Assert.Equal("state", otlpLogRecord.Body.StringValue);
}

#if EXPOSE_EXPERIMENTAL_FEATURES
[Fact]
public void CheckToOtlpLogRecordExceptionAttributes()
{
Expand Down Expand Up @@ -515,13 +526,14 @@ public void CheckToOtlpLogRecordExceptionAttributes()
Assert.Contains(SemanticConventions.AttributeExceptionStacktrace, otlpLogRecordAttributes);
Assert.Contains(logRecord.Exception.ToInvariantString(), otlpLogRecordAttributes);
}
#endif

[Fact]
public void CheckToOtlpLogRecordRespectsAttributeLimits()
{
var sdkLimitOptions = new SdkLimitOptions
{
AttributeCountLimit = 3, // 3 => LogCategory, exception.type and exception.message
AttributeCountLimit = 2,
AttributeValueLengthLimit = 8,
};

Expand All @@ -530,32 +542,33 @@ public void CheckToOtlpLogRecordRespectsAttributeLimits()
{
builder.AddOpenTelemetry(options =>
{
options.ParseStateValues = true;
options.AddInMemoryExporter(logRecords);
});
});

var logger = loggerFactory.CreateLogger("OtlpLogExporterTests");
logger.LogInformation(new NotSupportedException("I'm the exception message."), "Exception Occurred");
var logger = loggerFactory.CreateLogger(string.Empty);
logger.LogInformation("OpenTelemetry {AttributeOne} {AttributeTwo} {AttributeThree}!", "I'm an attribute", "I too am an attribute", "I get dropped :(");

var logRecord = logRecords[0];
var otlpLogRecord = logRecord.ToOtlpLog(sdkLimitOptions);

Assert.NotNull(otlpLogRecord);
Assert.Equal(1u, otlpLogRecord.DroppedAttributesCount);

var exceptionTypeAtt = TryGetAttribute(otlpLogRecord, SemanticConventions.AttributeExceptionType);
Assert.NotNull(exceptionTypeAtt);
var attribute = TryGetAttribute(otlpLogRecord, "AttributeOne");
Assert.NotNull(attribute);

// "NotSuppo" == first 8 chars from the exception typename "NotSupportedException"
Assert.Equal("NotSuppo", exceptionTypeAtt.Value.StringValue);
var exceptionMessageAtt = TryGetAttribute(otlpLogRecord, SemanticConventions.AttributeExceptionMessage);
Assert.NotNull(exceptionMessageAtt);
// "I'm an a" == first 8 chars from the first attribute "I'm an attribute"
Assert.Equal("I'm an a", attribute.Value.StringValue);
attribute = TryGetAttribute(otlpLogRecord, "AttributeTwo");
Assert.NotNull(attribute);

// "I'm the " == first 8 chars from the exception message
Assert.Equal("I'm the ", exceptionMessageAtt.Value.StringValue);
// "I too am" == first 8 chars from the second attribute "I too am an attribute"
Assert.Equal("I too am", attribute.Value.StringValue);

var exceptionStackTraceAtt = TryGetAttribute(otlpLogRecord, SemanticConventions.AttributeExceptionStacktrace);
Assert.Null(exceptionStackTraceAtt);
attribute = TryGetAttribute(otlpLogRecord, "AttributeThree");
Assert.Null(attribute);
}

[Fact]
Expand Down Expand Up @@ -670,7 +683,7 @@ public void ToOtlpLog_WhenOptionsIncludeScopesIsTrue_ContainsScopeAttributeStrin
options.AddInMemoryExporter(logRecords);
});
});
var logger = loggerFactory.CreateLogger(nameof(OtlpLogExporterTests));
var logger = loggerFactory.CreateLogger(string.Empty);

const string scopeKey = "Some scope key";

Expand All @@ -686,7 +699,7 @@ public void ToOtlpLog_WhenOptionsIncludeScopesIsTrue_ContainsScopeAttributeStrin
// Assert.
var logRecord = logRecords.Single();
var otlpLogRecord = logRecord.ToOtlpLog(DefaultSdkLimitOptions);
Assert.Equal(2, otlpLogRecord.Attributes.Count);
Assert.Single(otlpLogRecord.Attributes);
var actualScope = TryGetAttribute(otlpLogRecord, scopeKey);
Assert.NotNull(actualScope);
Assert.Equal(scopeKey, actualScope.Key);
Expand All @@ -709,7 +722,7 @@ public void ToOtlpLog_WhenOptionsIncludeScopesIsTrue_ContainsScopeAttributeBoolV
options.AddInMemoryExporter(logRecords);
});
});
var logger = loggerFactory.CreateLogger(nameof(OtlpLogExporterTests));
var logger = loggerFactory.CreateLogger(string.Empty);

const string scopeKey = "Some scope key";

Expand All @@ -725,7 +738,7 @@ public void ToOtlpLog_WhenOptionsIncludeScopesIsTrue_ContainsScopeAttributeBoolV
// Assert.
var logRecord = logRecords.Single();
var otlpLogRecord = logRecord.ToOtlpLog(DefaultSdkLimitOptions);
Assert.Equal(2, otlpLogRecord.Attributes.Count);
Assert.Single(otlpLogRecord.Attributes);
var actualScope = TryGetAttribute(otlpLogRecord, scopeKey);
Assert.NotNull(actualScope);
Assert.Equal(scopeKey, actualScope.Key);
Expand Down Expand Up @@ -760,7 +773,7 @@ public void ToOtlpLog_WhenOptionsIncludeScopesIsTrue_ContainsScopeAttributeIntVa
options.AddInMemoryExporter(logRecords);
});
});
var logger = loggerFactory.CreateLogger(nameof(OtlpLogExporterTests));
var logger = loggerFactory.CreateLogger(string.Empty);

const string scopeKey = "Some scope key";

Expand All @@ -776,7 +789,7 @@ public void ToOtlpLog_WhenOptionsIncludeScopesIsTrue_ContainsScopeAttributeIntVa
// Assert.
var logRecord = logRecords.Single();
var otlpLogRecord = logRecord.ToOtlpLog(DefaultSdkLimitOptions);
Assert.Equal(2, otlpLogRecord.Attributes.Count);
Assert.Single(otlpLogRecord.Attributes);
var actualScope = TryGetAttribute(otlpLogRecord, scopeKey);
Assert.NotNull(actualScope);
Assert.Equal(scopeKey, actualScope.Key);
Expand All @@ -799,7 +812,7 @@ public void ToOtlpLog_WhenOptionsIncludeScopesIsTrue_ContainsScopeAttributeDoubl
options.AddInMemoryExporter(logRecords);
});
});
var logger = loggerFactory.CreateLogger(nameof(OtlpLogExporterTests));
var logger = loggerFactory.CreateLogger(string.Empty);

const string scopeKey = "Some scope key";

Expand All @@ -815,7 +828,7 @@ public void ToOtlpLog_WhenOptionsIncludeScopesIsTrue_ContainsScopeAttributeDoubl
// Assert.
var logRecord = logRecords.Single();
var otlpLogRecord = logRecord.ToOtlpLog(DefaultSdkLimitOptions);
Assert.Equal(2, otlpLogRecord.Attributes.Count);
Assert.Single(otlpLogRecord.Attributes);
var actualScope = TryGetAttribute(otlpLogRecord, scopeKey);
Assert.NotNull(actualScope);
Assert.Equal(scopeKey, actualScope.Key);
Expand All @@ -838,7 +851,7 @@ public void ToOtlpLog_WhenOptionsIncludeScopesIsTrue_ContainsScopeAttributeDoubl
options.AddInMemoryExporter(logRecords);
});
});
var logger = loggerFactory.CreateLogger(nameof(OtlpLogExporterTests));
var logger = loggerFactory.CreateLogger(string.Empty);

const string scopeKey = "Some scope key";

Expand All @@ -854,7 +867,7 @@ public void ToOtlpLog_WhenOptionsIncludeScopesIsTrue_ContainsScopeAttributeDoubl
// Assert.
var logRecord = logRecords.Single();
var otlpLogRecord = logRecord.ToOtlpLog(DefaultSdkLimitOptions);
Assert.Equal(2, otlpLogRecord.Attributes.Count);
Assert.Single(otlpLogRecord.Attributes);
var actualScope = TryGetAttribute(otlpLogRecord, scopeKey);
Assert.NotNull(actualScope);
Assert.Equal(scopeKey, actualScope.Key);
Expand All @@ -874,7 +887,7 @@ public void ToOtlpLog_WhenOptionsIncludeScopesIsTrue_AndScopeStateIsOfTypeString
options.AddInMemoryExporter(logRecords);
});
});
var logger = loggerFactory.CreateLogger(nameof(OtlpLogExporterTests));
var logger = loggerFactory.CreateLogger(string.Empty);

const string scopeState = "Some scope state";

Expand All @@ -888,7 +901,7 @@ public void ToOtlpLog_WhenOptionsIncludeScopesIsTrue_AndScopeStateIsOfTypeString
var logRecord = logRecords.Single();
var otlpLogRecord = logRecord.ToOtlpLog(DefaultSdkLimitOptions);
Assert.NotNull(otlpLogRecord);
Assert.Single(otlpLogRecord.Attributes);
Assert.Empty(otlpLogRecord.Attributes);
}

[Theory]
Expand All @@ -909,7 +922,7 @@ public void ToOtlpLog_WhenOptionsIncludeScopesIsTrue_AndScopeStateIsOfPrimitiveT
options.AddInMemoryExporter(logRecords);
});
});
var logger = loggerFactory.CreateLogger(nameof(OtlpLogExporterTests));
var logger = loggerFactory.CreateLogger(string.Empty);

var scopeState = Activator.CreateInstance(typeOfScopeState);

Expand All @@ -923,7 +936,7 @@ public void ToOtlpLog_WhenOptionsIncludeScopesIsTrue_AndScopeStateIsOfPrimitiveT
var logRecord = logRecords.Single();
var otlpLogRecord = logRecord.ToOtlpLog(DefaultSdkLimitOptions);
Assert.NotNull(otlpLogRecord);
Assert.Single(otlpLogRecord.Attributes);
Assert.Empty(otlpLogRecord.Attributes);
}

[Fact]
Expand All @@ -939,7 +952,7 @@ public void ToOtlpLog_WhenOptionsIncludeScopesIsTrue_AndScopeStateIsOfDictionary
options.AddInMemoryExporter(logRecords);
});
});
var logger = loggerFactory.CreateLogger(nameof(OtlpLogExporterTests));
var logger = loggerFactory.CreateLogger(string.Empty);

const string scopeKey = "Some scope key";
const string scopeValue = "Some scope value";
Expand All @@ -954,7 +967,7 @@ public void ToOtlpLog_WhenOptionsIncludeScopesIsTrue_AndScopeStateIsOfDictionary
// Assert.
var logRecord = logRecords.Single();
var otlpLogRecord = logRecord.ToOtlpLog(DefaultSdkLimitOptions);
Assert.Equal(2, otlpLogRecord.Attributes.Count);
Assert.Single(otlpLogRecord.Attributes);
var actualScope = TryGetAttribute(otlpLogRecord, scopeKey);
Assert.NotNull(actualScope);
Assert.Equal(scopeKey, actualScope.Key);
Expand All @@ -977,7 +990,7 @@ public void ToOtlpLog_WhenOptionsIncludeScopesIsTrue_AndScopeStateIsOfEnumerable
options.AddInMemoryExporter(logRecords);
});
});
var logger = loggerFactory.CreateLogger(nameof(OtlpLogExporterTests));
var logger = loggerFactory.CreateLogger(string.Empty);

const string scopeKey = "Some scope key";
const string scopeValue = "Some scope value";
Expand All @@ -993,7 +1006,7 @@ public void ToOtlpLog_WhenOptionsIncludeScopesIsTrue_AndScopeStateIsOfEnumerable
// Assert.
var logRecord = logRecords.Single();
var otlpLogRecord = logRecord.ToOtlpLog(DefaultSdkLimitOptions);
Assert.Equal(2, otlpLogRecord.Attributes.Count);
Assert.Single(otlpLogRecord.Attributes);
var actualScope = TryGetAttribute(otlpLogRecord, scopeKey);
Assert.NotNull(actualScope);
Assert.Equal(scopeKey, actualScope.Key);
Expand All @@ -1015,7 +1028,7 @@ public void ToOtlpLog_WhenOptionsIncludeScopesIsTrue_AndMultipleScopesAreAdded_C
options.AddInMemoryExporter(logRecords);
});
});
var logger = loggerFactory.CreateLogger(nameof(OtlpLogExporterTests));
var logger = loggerFactory.CreateLogger(string.Empty);

const string scopeValue1 = "Some scope value";
const string scopeValue2 = "Some other scope value";
Expand All @@ -1036,7 +1049,7 @@ public void ToOtlpLog_WhenOptionsIncludeScopesIsTrue_AndMultipleScopesAreAdded_C
var allScopeValues = otlpLogRecord.Attributes
.Where(_ => _.Key == scopeKey1 || _.Key == scopeKey2)
.Select(_ => _.Value.StringValue);
Assert.Equal(3, otlpLogRecord.Attributes.Count);
Assert.Equal(2, otlpLogRecord.Attributes.Count);
Assert.Equal(2, allScopeValues.Count());
Assert.Contains(scopeValue1, allScopeValues);
Assert.Contains(scopeValue2, allScopeValues);
Expand All @@ -1057,7 +1070,7 @@ public void ToOtlpLog_WhenOptionsIncludeScopesIsTrue_AndMultipleScopeLevelsAreAd
options.AddInMemoryExporter(logRecords);
});
});
var logger = loggerFactory.CreateLogger(nameof(OtlpLogExporterTests));
var logger = loggerFactory.CreateLogger(string.Empty);

const string scopeValue1 = "Some scope value";
const string scopeValue2 = "Some other scope value";
Expand All @@ -1077,7 +1090,7 @@ public void ToOtlpLog_WhenOptionsIncludeScopesIsTrue_AndMultipleScopeLevelsAreAd
var allScopeValues = otlpLogRecord.Attributes
.Where(_ => _.Key == scopeKey1 || _.Key == scopeKey2)
.Select(_ => _.Value.StringValue);
Assert.Equal(3, otlpLogRecord.Attributes.Count);
Assert.Equal(2, otlpLogRecord.Attributes.Count);
Assert.Equal(2, allScopeValues.Count());
Assert.Contains(scopeValue1, allScopeValues);
Assert.Contains(scopeValue2, allScopeValues);
Expand All @@ -1098,7 +1111,7 @@ public void ToOtlpLog_WhenOptionsIncludeScopesIsTrue_AndScopeIsUsedInLogMethod_C
options.AddInMemoryExporter(logRecords);
});
});
var logger = loggerFactory.CreateLogger(nameof(OtlpLogExporterTests));
var logger = loggerFactory.CreateLogger(string.Empty);

const string scopeValue1 = "Some scope value";
const string scopeValue2 = "Some other scope value";
Expand All @@ -1111,9 +1124,9 @@ public void ToOtlpLog_WhenOptionsIncludeScopesIsTrue_AndScopeIsUsedInLogMethod_C
{
logger.Log(
LogLevel.Error,
new EventId(1),
default,
new List<KeyValuePair<string, object>> { new KeyValuePair<string, object>(scopeKey2, scopeValue2) },
exception: new Exception("Some exception message"),
exception: null,
formatter: (s, e) => string.Empty);
}

Expand All @@ -1123,7 +1136,7 @@ public void ToOtlpLog_WhenOptionsIncludeScopesIsTrue_AndScopeIsUsedInLogMethod_C
var allScopeValues = otlpLogRecord.Attributes
.Where(_ => _.Key == scopeKey1 || _.Key == scopeKey2)
.Select(_ => _.Value.StringValue);
Assert.Equal(7, otlpLogRecord.Attributes.Count);
Assert.Equal(2, otlpLogRecord.Attributes.Count);
Assert.Equal(2, allScopeValues.Count());
Assert.Contains(scopeValue1, allScopeValues);
Assert.Contains(scopeValue2, allScopeValues);
Expand Down