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

[Logs] Fix: Respect AttributeValueLengthLimit when building otlp LogRecord data #3684

Merged
Show file tree
Hide file tree
Changes from 3 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 @@ -2,6 +2,12 @@

## Unreleased

* Adds support for limiting the length and count of attributes exported from
the OTLP log exporter. These
[Attribute Limits](https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/sdk-environment-variables.md#attribute-limits)
are configured via the environment variables defined in the specification.
([#3684](https://github.com/open-telemetry/opentelemetry-dotnet/pull/3684))

## 1.4.0-beta.1

Released 2022-Sep-29
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@
using System.Collections.Generic;
using System.Runtime.CompilerServices;
using Google.Protobuf;
using Google.Protobuf.Collections;
using Microsoft.Extensions.Logging;
using OpenTelemetry.Configuration;
using OpenTelemetry.Internal;
using OpenTelemetry.Logs;
using OpenTelemetry.Trace;
Expand Down Expand Up @@ -75,14 +75,36 @@ internal static OtlpLogs.LogRecord ToOtlpLog(this LogRecord logRecord)
SeverityText = LogLevels[(int)logRecord.LogLevel],
};

var attributeValueLengthLimit = SdkConfiguration.Instance.AttributeValueLengthLimit;
var attributeCountLimit = SdkConfiguration.Instance.AttributeCountLimit ?? int.MaxValue;

// First add the generic attributes like category, eventid and exception, so they are less likely being dropped because of AttributeCountLimit

if (!string.IsNullOrEmpty(logRecord.CategoryName))
{
// TODO:
// 1. Track the following issue, and map CategoryName to Name
// if it makes it to log data model.
// https://github.com/open-telemetry/opentelemetry-specification/issues/2398
// 2. Confirm if this name for attribute is good.
otlpLogRecord.Attributes.AddStringAttribute("dotnet.ilogger.category", logRecord.CategoryName);
otlpLogRecord.AddStringAttribute("dotnet.ilogger.category", logRecord.CategoryName, attributeValueLengthLimit, attributeCountLimit);
}

if (logRecord.EventId.Id != default)
{
otlpLogRecord.AddIntAttribute(nameof(logRecord.EventId.Id), logRecord.EventId.Id, attributeValueLengthLimit, attributeCountLimit);
}

if (!string.IsNullOrEmpty(logRecord.EventId.Name))
{
otlpLogRecord.AddStringAttribute(nameof(logRecord.EventId.Name), logRecord.EventId.Name, attributeValueLengthLimit, attributeCountLimit);
}

if (logRecord.Exception != null)
{
otlpLogRecord.AddStringAttribute(SemanticConventions.AttributeExceptionType, logRecord.Exception.GetType().Name, attributeValueLengthLimit, attributeCountLimit);
otlpLogRecord.AddStringAttribute(SemanticConventions.AttributeExceptionMessage, logRecord.Exception.Message, attributeValueLengthLimit, attributeCountLimit);
otlpLogRecord.AddStringAttribute(SemanticConventions.AttributeExceptionStacktrace, logRecord.Exception.ToInvariantString(), attributeValueLengthLimit, attributeCountLimit);
}

bool bodyPopulatedFromFormattedMessage = false;
Expand All @@ -103,30 +125,13 @@ internal static OtlpLogs.LogRecord ToOtlpLog(this LogRecord logRecord)
{
otlpLogRecord.Body = new OtlpCommon.AnyValue { StringValue = stateValue.Value as string };
}
else if (OtlpKeyValueTransformer.Instance.TryTransformTag(stateValue, out var result))
else if (OtlpKeyValueTransformer.Instance.TryTransformTag(stateValue, out var result, attributeValueLengthLimit))
{
otlpLogRecord.Attributes.Add(result);
otlpLogRecord.AddAttribute(result, attributeCountLimit);
}
}
}

if (logRecord.EventId.Id != default)
{
otlpLogRecord.Attributes.AddIntAttribute(nameof(logRecord.EventId.Id), logRecord.EventId.Id);
}

if (!string.IsNullOrEmpty(logRecord.EventId.Name))
{
otlpLogRecord.Attributes.AddStringAttribute(nameof(logRecord.EventId.Name), logRecord.EventId.Name);
}

if (logRecord.Exception != null)
{
otlpLogRecord.Attributes.AddStringAttribute(SemanticConventions.AttributeExceptionType, logRecord.Exception.GetType().Name);
otlpLogRecord.Attributes.AddStringAttribute(SemanticConventions.AttributeExceptionMessage, logRecord.Exception.Message);
otlpLogRecord.Attributes.AddStringAttribute(SemanticConventions.AttributeExceptionStacktrace, logRecord.Exception.ToInvariantString());
}

if (logRecord.TraceId != default && logRecord.SpanId != default)
{
byte[] traceIdBytes = new byte[16];
Expand All @@ -149,9 +154,9 @@ void ProcessScope(LogRecordScope scope, OtlpLogs.LogRecord otlpLog)
foreach (var scopeItem in scope)
{
var scopeItemWithDepthInfo = new KeyValuePair<string, object>($"[Scope.{scopeDepth}]:{scopeItem.Key}", scopeItem.Value);
if (OtlpKeyValueTransformer.Instance.TryTransformTag(scopeItemWithDepthInfo, out var result))
if (OtlpKeyValueTransformer.Instance.TryTransformTag(scopeItemWithDepthInfo, out var result, attributeValueLengthLimit))
{
otlpLog.Attributes.Add(result);
otlpLog.AddAttribute(result, attributeCountLimit);
}
}
}
Expand All @@ -164,22 +169,37 @@ void ProcessScope(LogRecordScope scope, OtlpLogs.LogRecord otlpLog)
return otlpLogRecord;
}

private static void AddStringAttribute(this RepeatedField<OtlpCommon.KeyValue> repeatedField, string key, string value)
[MethodImpl(MethodImplOptions.AggressiveInlining)]
private static void AddAttribute(this OtlpLogs.LogRecord logRecord, OtlpCommon.KeyValue attribute, int maxAttributeCount)
{
repeatedField.Add(new OtlpCommon.KeyValue
if (logRecord.Attributes.Count < maxAttributeCount)
Copy link
Member

Choose a reason for hiding this comment

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

@alanwest One random thought looking at this PR about how we could improve things...

We build the OtlpCommon.KeyValue before we check if we will drop it. Could probably improve that so we skip the allocation(s) if we know we are up against the limit?

Copy link
Contributor Author

@JWilh JWilh Oct 14, 2022

Choose a reason for hiding this comment

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

@alanwest I just tried changing it for the LogRecord extension, but we would need something likely a dryrun for TryTransformTag or second function, otherwise we would count attributes as dropped, that were ignored by TryTransformTag before.

{
logRecord.Attributes.Add(attribute);
}
else
{
Key = key,
Value = new OtlpCommon.AnyValue { StringValue = value },
});
logRecord.DroppedAttributesCount++;
}
}

private static void AddIntAttribute(this RepeatedField<OtlpCommon.KeyValue> repeatedField, string key, int value)
[MethodImpl(MethodImplOptions.AggressiveInlining)]
private static void AddStringAttribute(this OtlpLogs.LogRecord logRecord, string key, string value, int? maxValueLength, int maxAttributeCount)
{
repeatedField.Add(new OtlpCommon.KeyValue
var attributeItem = new KeyValuePair<string, object>(key, value);
if (OtlpKeyValueTransformer.Instance.TryTransformTag(attributeItem, out var result, maxValueLength))
{
Key = key,
Value = new OtlpCommon.AnyValue { IntValue = value },
});
logRecord.AddAttribute(result, maxAttributeCount);
}
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
private static void AddIntAttribute(this OtlpLogs.LogRecord logRecord, string key, int value, int? maxValueLength, int maxAttributeCount)
JWilh marked this conversation as resolved.
Show resolved Hide resolved
{
var attributeItem = new KeyValuePair<string, object>(key, value);
if (OtlpKeyValueTransformer.Instance.TryTransformTag(attributeItem, out var result, maxValueLength))
{
logRecord.AddAttribute(result, maxAttributeCount);
}
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
Expand Down