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.logging] More consistent handling of StateValues/Attributes #4334

Merged
merged 22 commits into from
Apr 20, 2023
Merged
Show file tree
Hide file tree
Changes from 20 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
10 changes: 2 additions & 8 deletions docs/logs/redaction/MyRedactionProcessor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -23,15 +23,9 @@ internal class MyRedactionProcessor : BaseProcessor<LogRecord>
{
public override void OnEnd(LogRecord logRecord)
{
if (logRecord.State == null)
if (logRecord.Attributes != null)
Copy link
Member

Choose a reason for hiding this comment

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

adding a comment to the PR for anyone wondering: This change is not required really (except to silence warning about using obsolete. The change is backcompatible, so if a Redactor is looking at State or Statealues as before, it'll continue to work, and there is no risk of "Oh damn, I missed to redact secret attributes".

{
// When State is null, OTel SDK guarantees StateValues is populated
// TODO: Debug.Assert?
logRecord.StateValues = new MyClassWithRedactionEnumerator(logRecord.StateValues);
}
else if (logRecord.State is IReadOnlyList<KeyValuePair<string, object>> listOfKvp)
{
logRecord.State = new MyClassWithRedactionEnumerator(listOfKvp);
logRecord.Attributes = new MyClassWithRedactionEnumerator(logRecord.Attributes);
}
}
}
5 changes: 5 additions & 0 deletions src/OpenTelemetry.Exporter.Console/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,11 @@
* Added direct reference to `System.Text.Encodings.Web` with minimum version of
`4.7.2` in response to [CVE-2021-26701](https://github.com/dotnet/runtime/issues/49377).

* Updated `LogRecord` console output: `Body` is now shown (if set),
`StateValues` are now written as `Attributes`, and `State` is no longer
processed.
([#4334](https://github.com/open-telemetry/opentelemetry-dotnet/pull/4334))

## 1.5.0-alpha.2

Released 2023-Mar-31
Expand Down
38 changes: 9 additions & 29 deletions src/OpenTelemetry.Exporter.Console/ConsoleLogRecordExporter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -81,42 +81,22 @@ public override ExportResult Export(in Batch<LogRecord> batch)
this.WriteLine($"{"LogRecord.FormattedMessage:",-RightPaddingLength}{logRecord.FormattedMessage}");
}

if (logRecord.State != null)
if (logRecord.Body != null)
{
if (logRecord.State is IReadOnlyList<KeyValuePair<string, object>> listKvp)
{
this.WriteLine("LogRecord.State (Key:Value):");
for (int i = 0; i < listKvp.Count; i++)
{
// Special casing {OriginalFormat}
// See https://github.com/open-telemetry/opentelemetry-dotnet/pull/3182
// for explanation.
var valueToTransform = listKvp[i].Key.Equals("{OriginalFormat}")
? new KeyValuePair<string, object>("OriginalFormat (a.k.a Body)", listKvp[i].Value)
: listKvp[i];

if (ConsoleTagTransformer.Instance.TryTransformTag(valueToTransform, out var result))
{
this.WriteLine($"{string.Empty,-4}{result}");
}
}
}
else
{
this.WriteLine($"{"LogRecord.State:",-RightPaddingLength}{logRecord.State}");
}
this.WriteLine($"{"LogRecord.Body:",-RightPaddingLength}{logRecord.Body}");
}
else if (logRecord.StateValues != null)

if (logRecord.Attributes != null)
{
this.WriteLine("LogRecord.StateValues (Key:Value):");
for (int i = 0; i < logRecord.StateValues.Count; i++)
this.WriteLine("LogRecord.Attributes (Key:Value):");
for (int i = 0; i < logRecord.Attributes.Count; i++)
{
// Special casing {OriginalFormat}
// See https://github.com/open-telemetry/opentelemetry-dotnet/pull/3182
// for explanation.
var valueToTransform = logRecord.StateValues[i].Key.Equals("{OriginalFormat}")
? new KeyValuePair<string, object>("OriginalFormat (a.k.a Body)", logRecord.StateValues[i].Value)
: logRecord.StateValues[i];
var valueToTransform = logRecord.Attributes[i].Key.Equals("{OriginalFormat}")
? new KeyValuePair<string, object>("OriginalFormat (a.k.a Body)", logRecord.Attributes[i].Value)
: logRecord.Attributes[i];

if (ConsoleTagTransformer.Instance.TryTransformTag(valueToTransform, out var result))
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,14 @@

## Unreleased

* The `OpenTelemetryLoggerOptions.AddOtlpExporter` extension no longer
CodeBlanch marked this conversation as resolved.
Show resolved Hide resolved
automatically set `OpenTelemetryLoggerOptions.ParseStateValues` to `true`.
([#4334](https://github.com/open-telemetry/opentelemetry-dotnet/pull/4334))
CodeBlanch marked this conversation as resolved.
Show resolved Hide resolved

* Updated to use the new `LogRecord.Attributes` field as `StateValues` is now
marked obsolete.
([#4334](https://github.com/open-telemetry/opentelemetry-dotnet/pull/4334))

## 1.5.0-alpha.2

Released 2023-Mar-31
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,6 @@ public static OpenTelemetryLoggerOptions AddOtlpExporter(this OpenTelemetryLogge
/// <summary>
/// Adds OTLP Exporter as a configuration to the OpenTelemetry ILoggingBuilder.
/// </summary>
/// <remarks>
/// Note: AddOtlpExporter automatically sets <see
/// cref="OpenTelemetryLoggerOptions.ParseStateValues"/> to <see
/// langword="true"/>.
/// </remarks>
/// <param name="loggerOptions"><see cref="OpenTelemetryLoggerOptions"/> options to use.</param>
/// <param name="configure">Callback action for configuring <see cref="OtlpExporterOptions"/>.</param>
/// <returns>The instance of <see cref="OpenTelemetryLoggerOptions"/> to chain the calls.</returns>
Expand All @@ -53,8 +48,6 @@ private static OpenTelemetryLoggerOptions AddOtlpExporterInternal(
OpenTelemetryLoggerOptions loggerOptions,
Action<OtlpExporterOptions> configure)
{
loggerOptions.ParseStateValues = true;
cijothomas marked this conversation as resolved.
Show resolved Hide resolved

var exporterOptions = new OtlpExporterOptions();

// TODO: We are using span/activity batch environment variable keys
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,18 +113,18 @@ internal static OtlpLogs.LogRecord ToOtlpLog(this LogRecord logRecord, SdkLimitO
bodyPopulatedFromFormattedMessage = true;
}

if (logRecord.StateValues != null)
if (logRecord.Attributes != null)
{
foreach (var stateValue in logRecord.StateValues)
foreach (var attribute in logRecord.Attributes)
{
// Special casing {OriginalFormat}
// See https://github.com/open-telemetry/opentelemetry-dotnet/pull/3182
// for explanation.
if (stateValue.Key.Equals("{OriginalFormat}") && !bodyPopulatedFromFormattedMessage)
if (attribute.Key.Equals("{OriginalFormat}") && !bodyPopulatedFromFormattedMessage)
{
otlpLogRecord.Body = new OtlpCommon.AnyValue { StringValue = stateValue.Value as string };
otlpLogRecord.Body = new OtlpCommon.AnyValue { StringValue = attribute.Value as string };
}
else if (OtlpKeyValueTransformer.Instance.TryTransformTag(stateValue, out var result, attributeValueLengthLimit))
else if (OtlpKeyValueTransformer.Instance.TryTransformTag(attribute, out var result, attributeValueLengthLimit))
{
otlpLogRecord.AddAttribute(result, attributeCountLimit);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
OpenTelemetry.Logs.LogRecord.Attributes.get -> System.Collections.Generic.IReadOnlyList<System.Collections.Generic.KeyValuePair<string!, object?>>?
OpenTelemetry.Logs.LogRecord.Attributes.set -> void
OpenTelemetry.Logs.LogRecord.Body.get -> string?
OpenTelemetry.Logs.LogRecord.Body.set -> void
OpenTelemetry.Metrics.AlwaysOffExemplarFilter
OpenTelemetry.Metrics.AlwaysOffExemplarFilter.AlwaysOffExemplarFilter() -> void
OpenTelemetry.Metrics.AlwaysOnExemplarFilter
Expand Down Expand Up @@ -43,4 +47,4 @@ OpenTelemetry.Metrics.MetricPoint.GetExponentialHistogramData() -> OpenTelemetry
~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!
OpenTelemetry.Resources.ResourceBuilder.AddDetector(System.Func<System.IServiceProvider!, OpenTelemetry.Resources.IResourceDetector!>! resourceDetectorFactory) -> OpenTelemetry.Resources.ResourceBuilder!
4 changes: 4 additions & 0 deletions src/OpenTelemetry/.publicApi/net6.0/PublicAPI.Unshipped.txt
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
OpenTelemetry.Logs.LogRecord.Attributes.get -> System.Collections.Generic.IReadOnlyList<System.Collections.Generic.KeyValuePair<string!, object?>>?
OpenTelemetry.Logs.LogRecord.Attributes.set -> void
OpenTelemetry.Logs.LogRecord.Body.get -> string?
OpenTelemetry.Logs.LogRecord.Body.set -> void
OpenTelemetry.Metrics.AlwaysOffExemplarFilter
OpenTelemetry.Metrics.AlwaysOffExemplarFilter.AlwaysOffExemplarFilter() -> void
OpenTelemetry.Metrics.AlwaysOnExemplarFilter
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
OpenTelemetry.Logs.LogRecord.Attributes.get -> System.Collections.Generic.IReadOnlyList<System.Collections.Generic.KeyValuePair<string!, object?>>?
OpenTelemetry.Logs.LogRecord.Attributes.set -> void
OpenTelemetry.Logs.LogRecord.Body.get -> string?
OpenTelemetry.Logs.LogRecord.Body.set -> void
OpenTelemetry.Metrics.AlwaysOffExemplarFilter
OpenTelemetry.Metrics.AlwaysOffExemplarFilter.AlwaysOffExemplarFilter() -> void
OpenTelemetry.Metrics.AlwaysOnExemplarFilter
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
OpenTelemetry.Logs.LogRecord.Attributes.get -> System.Collections.Generic.IReadOnlyList<System.Collections.Generic.KeyValuePair<string!, object?>>?
OpenTelemetry.Logs.LogRecord.Attributes.set -> void
OpenTelemetry.Logs.LogRecord.Body.get -> string?
OpenTelemetry.Logs.LogRecord.Body.set -> void
OpenTelemetry.Metrics.AlwaysOffExemplarFilter
OpenTelemetry.Metrics.AlwaysOffExemplarFilter.AlwaysOffExemplarFilter() -> void
OpenTelemetry.Metrics.AlwaysOnExemplarFilter
Expand Down
7 changes: 7 additions & 0 deletions src/OpenTelemetry/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,20 @@
the `telemetry.sdk.*` attributes defined in the
[specification](https://github.com/open-telemetry/opentelemetry-specification/tree/12fcec1ff255b1535db75708e52a3a21f86f0fae/specification/resource/semantic_conventions#semantic-attributes-with-sdk-provided-default-value).
([#4369](https://github.com/open-telemetry/opentelemetry-dotnet/pull/4369))

* Fixed an issue with `HashCode` computations throwing exceptions on .NET
Standard 2.1 targets.
([#4362](https://github.com/open-telemetry/opentelemetry-dotnet/pull/4362))

* Update value of the resource attribute `telemetry.sdk.version` to show the tag
name which resembles the package version of the SDK.
([#4375](https://github.com/open-telemetry/opentelemetry-dotnet/pull/4375))

* Tweaked the behavior of the `OpenTelemetryLoggerOptions.ParseStateValues`
CodeBlanch marked this conversation as resolved.
Show resolved Hide resolved
flag, obsoleted `LogRecord.State` and `LogRecord.StateValues` properties, and
added `LogRecord.Body` and `LogRecord.Attributes` properties.
([#4334](https://github.com/open-telemetry/opentelemetry-dotnet/pull/4334))

## 1.5.0-alpha.2

Released 2023-Mar-31
Expand Down
15 changes: 15 additions & 0 deletions src/OpenTelemetry/Internal/OpenTelemetrySdkEventSource.cs
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,15 @@ public void DroppedExportProcessorItems(string exportProcessorName, string expor
}
}

[NonEvent]
public void LoggerParseStateException<TState>(Exception exception)
{
if (this.IsEnabled(EventLevel.Warning, EventKeywords.All))
{
this.LoggerParseStateException(typeof(TState).FullName!, exception.ToInvariantString());
}
}

[Event(4, Message = "Unknown error in SpanProcessor event '{0}': '{1}'.", Level = EventLevel.Error)]
public void SpanProcessorException(string evnt, string ex)
{
Expand Down Expand Up @@ -284,6 +293,12 @@ public void InvalidEnvironmentVariable(string key, string? value)
this.WriteEvent(47, key, value);
}

[Event(48, Message = "Exception thrown parsing log state of type '{0}'. Exception: '{1}'", Level = EventLevel.Warning)]
public void LoggerParseStateException(string type, string error)
{
this.WriteEvent(48, type, error);
}

#if DEBUG
public class OpenTelemetryEventListener : EventListener
{
Expand Down
Loading