Skip to content

[OpenTelemetry.Api] Resolve CA1815 #6595

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
+493 −7
Open

[OpenTelemetry.Api] Resolve CA1815 #6595

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7293010
[OpenTelemetry.Api] Resolve CA1815
martincostello Oct 9, 2025
9197288
[OpenTelemetry.Api] Add tests
martincostello Oct 9, 2025
ed8165c
Merge branch 'main' into gh-6278
rajkumar-rangaraj Oct 10, 2025
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: 10 additions & 0 deletions src/OpenTelemetry.Api/.publicApi/Experimental/PublicAPI.Unshipped.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
[OTEL1001]OpenTelemetry.Logs.LogRecordAttributeList.Enumerator.Dispose() -> void
[OTEL1001]OpenTelemetry.Logs.LogRecordAttributeList.Enumerator.Enumerator() -> void
[OTEL1001]OpenTelemetry.Logs.LogRecordAttributeList.Enumerator.MoveNext() -> bool
[OTEL1001]OpenTelemetry.Logs.LogRecordAttributeList.Equals(OpenTelemetry.Logs.LogRecordAttributeList other) -> bool
[OTEL1001]OpenTelemetry.Logs.LogRecordAttributeList.GetEnumerator() -> OpenTelemetry.Logs.LogRecordAttributeList.Enumerator
[OTEL1001]OpenTelemetry.Logs.LogRecordAttributeList.LogRecordAttributeList() -> void
[OTEL1001]OpenTelemetry.Logs.LogRecordAttributeList.RecordException(System.Exception! exception) -> void
Expand All @@ -26,6 +27,7 @@
[OTEL1001]OpenTelemetry.Logs.LogRecordData
[OTEL1001]OpenTelemetry.Logs.LogRecordData.Body.get -> string?
[OTEL1001]OpenTelemetry.Logs.LogRecordData.Body.set -> void
[OTEL1001]OpenTelemetry.Logs.LogRecordData.Equals(OpenTelemetry.Logs.LogRecordData other) -> bool
[OTEL1001]OpenTelemetry.Logs.LogRecordData.LogRecordData() -> void
[OTEL1001]OpenTelemetry.Logs.LogRecordData.LogRecordData(in System.Diagnostics.ActivityContext activityContext) -> void
[OTEL1001]OpenTelemetry.Logs.LogRecordData.LogRecordData(System.Diagnostics.Activity? activity) -> void
Expand Down Expand Up @@ -70,6 +72,14 @@
[OTEL1001]OpenTelemetry.Logs.LogRecordSeverity.Warn3 = 15 -> OpenTelemetry.Logs.LogRecordSeverity
[OTEL1001]OpenTelemetry.Logs.LogRecordSeverity.Warn4 = 16 -> OpenTelemetry.Logs.LogRecordSeverity
[OTEL1001]OpenTelemetry.Logs.LogRecordSeverityExtensions
[OTEL1001]override OpenTelemetry.Logs.LogRecordAttributeList.Equals(object? obj) -> bool
Copy link
Member

Choose a reason for hiding this comment

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

Do you have a strong reason why you want to increase the public API surface to remove the CA1815 suppression from the code?
CA1815 is a performance-oriented warning, but implementing Equals, GetHashCode, and equality operators (==, !=) on these structs would expose them as public APIs. This means:

  1. API stability commitment - Once these are public, we're committed to maintaining them
  2. Use case unclear - Are users actually comparing these structs or using them in hash-based collections where the default implementation would be problematic?
  3. Suppression is valid - If these structs aren't intended to be compared or hashed, suppressing CA1815 is the appropriate choice

If there's no demonstrated performance issue or user scenario requiring these equality members, it's better to keep the suppression rather than expanding the public API surface. Can you share the specific use case that necessitates implementing these methods?

Copy link
Member Author

Choose a reason for hiding this comment

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

I defer to the linked issue which tracks doing the work to remove the suppressions.

Copy link
Member

Choose a reason for hiding this comment

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

I have created this issue while working on enabling <AnalysisLevel>latest-all</AnalysisLevel> across whole repository.
Touching public API was fully out of scope of this task so I created this one. I suppose that Raj's comment might be important, but it is still going case by case if it is worth to add all these methods. If not, it will be great to extend warning comments that is it no needed, and perf was tested.

[OTEL1001]override OpenTelemetry.Logs.LogRecordAttributeList.GetHashCode() -> int
[OTEL1001]override OpenTelemetry.Logs.LogRecordData.Equals(object? obj) -> bool
[OTEL1001]override OpenTelemetry.Logs.LogRecordData.GetHashCode() -> int
[OTEL1001]static OpenTelemetry.Logs.LogRecordAttributeList.CreateFromEnumerable(System.Collections.Generic.IEnumerable<System.Collections.Generic.KeyValuePair<string!, object?>>! attributes) -> OpenTelemetry.Logs.LogRecordAttributeList
[OTEL1001]static OpenTelemetry.Logs.LogRecordAttributeList.operator !=(OpenTelemetry.Logs.LogRecordAttributeList left, OpenTelemetry.Logs.LogRecordAttributeList right) -> bool
[OTEL1001]static OpenTelemetry.Logs.LogRecordAttributeList.operator ==(OpenTelemetry.Logs.LogRecordAttributeList left, OpenTelemetry.Logs.LogRecordAttributeList right) -> bool
[OTEL1001]static OpenTelemetry.Logs.LogRecordData.operator !=(OpenTelemetry.Logs.LogRecordData left, OpenTelemetry.Logs.LogRecordData right) -> bool
[OTEL1001]static OpenTelemetry.Logs.LogRecordData.operator ==(OpenTelemetry.Logs.LogRecordData left, OpenTelemetry.Logs.LogRecordData right) -> bool
[OTEL1001]static OpenTelemetry.Logs.LogRecordSeverityExtensions.ToShortName(this OpenTelemetry.Logs.LogRecordSeverity logRecordSeverity) -> string!
[OTEL1001]virtual OpenTelemetry.Logs.LoggerProvider.TryCreateLogger(string? name, out OpenTelemetry.Logs.Logger? logger) -> bool
78 changes: 75 additions & 3 deletions src/OpenTelemetry.Api/Logs/LogRecordAttributeList.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,7 @@ namespace OpenTelemetry.Logs;
/// </summary>
internal
#endif
#pragma warning disable CA1815 // Override equals and operator equals on value types
struct LogRecordAttributeList : IReadOnlyList<KeyValuePair<string, object?>>
#pragma warning restore CA1815 // Override equals and operator equals on value types
struct LogRecordAttributeList : IReadOnlyList<KeyValuePair<string, object?>>, IEquatable<LogRecordAttributeList>
{
internal const int OverflowMaxCount = 8;
internal const int OverflowAdditionalCapacity = 16;
Expand Down Expand Up @@ -122,6 +120,26 @@ public object? this[string key]
set => this.Add(new KeyValuePair<string, object?>(key, value));
}

/// <summary>
/// Determines whether the two instances of <see cref="LogRecordAttributeList"/> are equal.
/// </summary>
/// <param name="left">An instance of <see cref="LogRecordAttributeList"/>.</param>
/// <param name="right"> Another instance of <see cref="LogRecordAttributeList"/>.</param>
/// <returns>
/// <see langword="true"/> if the values are considered equal; otherwise, <see langword="false"/>.
/// </returns>
public static bool operator ==(LogRecordAttributeList left, LogRecordAttributeList right) => left.Equals(right);

/// <summary>
/// Determines whether the two instances of <see cref="LogRecordAttributeList"/> are not equal.
/// </summary>
/// <param name="left">An instance of <see cref="LogRecordAttributeList"/>.</param>
/// <param name="right"> Another instance of <see cref="LogRecordAttributeList"/>.</param>
/// <returns>
/// <see langword="true"/> if the values are not considered equal; otherwise, <see langword="false"/>.
/// </returns>
public static bool operator !=(LogRecordAttributeList left, LogRecordAttributeList right) => !(left == right);

/// <summary>
/// Create a <see cref="LogRecordAttributeList"/> collection from an enumerable.
/// </summary>
Expand All @@ -137,6 +155,60 @@ public static LogRecordAttributeList CreateFromEnumerable(IEnumerable<KeyValuePa
return logRecordAttributes;
}

/// <inheritdoc/>
public override readonly bool Equals(object? obj) =>
obj is LogRecordAttributeList other && this.Equals(other);

/// <inheritdoc/>
public readonly bool Equals(LogRecordAttributeList other)
{
if (this.count != other.count)
{
return false;
}

for (int i = 0; i < this.count; i++)
{
if (!this[i].Equals(other[i]))
{
return false;
}
}

return true;
}

/// <inheritdoc/>
public override readonly int GetHashCode()
{
#if NET
HashCode combined = default;

for (int i = 0; i < this.count; i++)
{
var item = this[i];
combined.Add(item.Key);
combined.Add(item.Value);
}

return combined.ToHashCode();
#else
unchecked
{
var hash = 17;

for (int i = 0; i < this.count; i++)
{
var item = this[i];
hash = (hash * 31) + item.Key.GetHashCode();
hash = (hash * 31) + (item.Value?.GetHashCode() ?? 0);
}

return hash;
}
#endif
}

/// <summary>
/// Add an attribute.
/// </summary>
Expand Down
71 changes: 67 additions & 4 deletions src/OpenTelemetry.Api/Logs/LogRecordData.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,7 @@ namespace OpenTelemetry.Logs;
/// </summary>
internal
#endif
#pragma warning disable CA1815 // Override equals and operator equals on value types
struct LogRecordData
#pragma warning restore CA1815 // Override equals and operator equals on value types
struct LogRecordData : IEquatable<LogRecordData>
{
internal DateTime TimestampBacking = DateTime.UtcNow;

Expand Down Expand Up @@ -89,7 +87,7 @@ public LogRecordData(in ActivityContext activityContext)
public DateTime Timestamp
{
readonly get => this.TimestampBacking;
set { this.TimestampBacking = value.Kind == DateTimeKind.Local ? value.ToUniversalTime() : value; }
set => this.TimestampBacking = value.Kind == DateTimeKind.Local ? value.ToUniversalTime() : value;
}

/// <summary>
Expand Down Expand Up @@ -128,6 +126,71 @@ public DateTime Timestamp
/// </summary>
public string? EventName { get; set; } = null;

/// <summary>
/// Determines whether the two instances of <see cref="LogRecordData"/> are equal.
/// </summary>
/// <param name="left">An instance of <see cref="LogRecordData"/>.</param>
/// <param name="right"> Another instance of <see cref="LogRecordData"/>.</param>
/// <returns>
/// <see langword="true"/> if the values are considered equal; otherwise, <see langword="false"/>.
/// </returns>
public static bool operator ==(LogRecordData left, LogRecordData right) => left.Equals(right);

/// <summary>
/// Determines whether the two instances of <see cref="LogRecordData"/> are not equal.
/// </summary>
/// <param name="left">An instance of <see cref="LogRecordData"/>.</param>
/// <param name="right"> Another instance of <see cref="LogRecordData"/>.</param>
/// <returns>
/// <see langword="true"/> if the values are not considered equal; otherwise, <see langword="false"/>.
/// </returns>
public static bool operator !=(LogRecordData left, LogRecordData right) => !(left == right);

/// <inheritdoc/>
public override readonly bool Equals(object? obj) =>
obj is LogRecordData other && this.Equals(other);

/// <inheritdoc/>
public readonly bool Equals(LogRecordData other) =>
this.TimestampBacking == other.TimestampBacking &&
this.TraceId == other.TraceId &&
this.SpanId == other.SpanId &&
this.TraceFlags == other.TraceFlags &&
this.Severity == other.Severity &&
this.SeverityText == other.SeverityText &&
this.Body == other.Body &&
this.EventName == other.EventName;

/// <inheritdoc/>
public override readonly int GetHashCode()
#if NET
=> HashCode.Combine(
this.TimestampBacking,
this.TraceId,
this.SpanId,
this.TraceFlags,
this.Severity,
this.SeverityText,
this.Body,
this.EventName);
#else
{
unchecked
{
var hash = 17;
hash = (hash * 31) + this.TimestampBacking.GetHashCode();
hash = (hash * 31) + this.TraceId.GetHashCode();
hash = (hash * 31) + this.SpanId.GetHashCode();
hash = (hash * 31) + this.TraceFlags.GetHashCode();
hash = (hash * 31) + (this.Severity?.GetHashCode() ?? 0);
hash = (hash * 31) + (this.SeverityText?.GetHashCode() ?? 0);
hash = (hash * 31) + (this.Body?.GetHashCode() ?? 0);
hash = (hash * 31) + (this.EventName?.GetHashCode() ?? 0);
return hash;
}
}
#endif

internal static void SetActivityContext(ref LogRecordData data, Activity? activity)
{
if (activity != null)
Expand Down
143 changes: 143 additions & 0 deletions test/OpenTelemetry.Api.Tests/Logs/LogRecordAttributeListTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,4 +157,147 @@ public void InitializerIndexesSyntaxTest()

Assert.Equal(2, list.Count);
}

[Fact]
public void Equals_Object_ReturnsTrueForSameAttributes()
{
var left = new LogRecordAttributeList
{
["key1"] = "value1",
["key2"] = 123,
};

var right = new LogRecordAttributeList
{
["key1"] = "value1",
["key2"] = 123,
};

Assert.True(left.Equals((object)right));
Assert.True(((object)left).Equals(right));
}

[Fact]
public void Equals_Object_ReturnsFalseForDifferentAttributes()
{
var left = new LogRecordAttributeList
{
["key1"] = "value1",
["key2"] = 123,
};

var right = new LogRecordAttributeList
{
["key1"] = "value2",
["key2"] = 123,
};

Assert.False(left.Equals((object)right));
Assert.False(((object)left).Equals(right));
}

[Fact]
public void Equals_Object_ReturnsFalseForAnotherType()
{
var left = new LogRecordAttributeList
{
["key1"] = "value1",
["key2"] = 123,
};

var right = "foo";

Assert.False(left.Equals(right));
}

[Fact]
public void Equals_Typed_ReturnsTrueForSameAttributes()
{
var left = new LogRecordAttributeList
{
["a"] = 1,
};

var right = new LogRecordAttributeList
{
["a"] = 1,
};

Assert.True(left.Equals(right));
}

[Fact]
public void Equals_Typed_ReturnsFalseForDifferentCount()
{
var left = new LogRecordAttributeList
{
["a"] = 1,
};

var right = default(LogRecordAttributeList);

Assert.False(left.Equals(right));
}

[Fact]
public void Operator_Equality_ReturnsTrueForEqualLists()
{
var left = new LogRecordAttributeList
{
["x"] = 42,
};
var right = new LogRecordAttributeList
{
["x"] = 42,
};

Assert.True(left == right);
Assert.False(left != right);
}

[Fact]
public void Operator_Equality_ReturnsFalseForDifferentLists()
{
var left = new LogRecordAttributeList
{
["x"] = 42,
};
var right = new LogRecordAttributeList
{
["x"] = 43,
};

Assert.False(left == right);
Assert.True(left != right);
}

[Fact]
public void GetHashCode_SameForEqualLists()
{
var left = new LogRecordAttributeList
{
["foo"] = "bar",
};
var right = new LogRecordAttributeList
{
["foo"] = "bar",
};

Assert.Equal(left.GetHashCode(), right.GetHashCode());
}

[Fact]
public void GetHashCode_DifferentForDifferentLists()
{
var left = new LogRecordAttributeList
{
["foo"] = "bar",
};
var right = new LogRecordAttributeList
{
["foo"] = "baz",
};

Assert.NotEqual(left.GetHashCode(), right.GetHashCode());
}
}
Loading
Loading