[Propagators] Nullable annotations #5767
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
github-actions
bot
added
pkg:OpenTelemetry.Api
|
Potential diff for contrib repo: diff --git a/src/OpenTelemetry.Extensions.AWS/.publicApi/PublicAPI.Unshipped.txt b/src/OpenTelemetry.Extensions.AWS/.publicApi/PublicAPI.Unshipped.txt
index a7a1588b..cc69db22 100644
--- a/src/OpenTelemetry.Extensions.AWS/.publicApi/PublicAPI.Unshipped.txt
+++ b/src/OpenTelemetry.Extensions.AWS/.publicApi/PublicAPI.Unshipped.txt
@@ -2,7 +2,7 @@ OpenTelemetry.Extensions.AWS.AWSXRayIdGenerator
OpenTelemetry.Extensions.AWS.Trace.AWSXRayPropagator
OpenTelemetry.Extensions.AWS.Trace.AWSXRayPropagator.AWSXRayPropagator() -> void
OpenTelemetry.Trace.TracerProviderBuilderExtensions
-override OpenTelemetry.Extensions.AWS.Trace.AWSXRayPropagator.Extract<T>(OpenTelemetry.Context.Propagation.PropagationContext context, T carrier, System.Func<T, string!, System.Collections.Generic.IEnumerable<string!>!>! getter) -> OpenTelemetry.Context.Propagation.PropagationContext
+override OpenTelemetry.Extensions.AWS.Trace.AWSXRayPropagator.Extract<T>(OpenTelemetry.Context.Propagation.PropagationContext context, T carrier, System.Func<T, string!, System.Collections.Generic.IEnumerable<string!>?>! getter) -> OpenTelemetry.Context.Propagation.PropagationContext
override OpenTelemetry.Extensions.AWS.Trace.AWSXRayPropagator.Fields.get -> System.Collections.Generic.ISet<string!>!
override OpenTelemetry.Extensions.AWS.Trace.AWSXRayPropagator.Inject<T>(OpenTelemetry.Context.Propagation.PropagationContext context, T carrier, System.Action<T, string!, string!>! setter) -> void
static OpenTelemetry.Trace.TracerProviderBuilderExtensions.AddXRayTraceId(this OpenTelemetry.Trace.TracerProviderBuilder! builder) -> OpenTelemetry.Trace.TracerProviderBuilder!
diff --git a/src/OpenTelemetry.Extensions.AWS/Trace/AWSXRayPropagator.cs b/src/OpenTelemetry.Extensions.AWS/Trace/AWSXRayPropagator.cs
index 569bae53..4e8d5ac8 100644
--- a/src/OpenTelemetry.Extensions.AWS/Trace/AWSXRayPropagator.cs
+++ b/src/OpenTelemetry.Extensions.AWS/Trace/AWSXRayPropagator.cs
@@ -38,7 +38,7 @@ public class AWSXRayPropagator : TextMapPropagator
public override ISet<string> Fields => new HashSet<string>() { AWSXRayTraceHeaderKey };
/// <inheritdoc/>
- public override PropagationContext Extract<T>(PropagationContext context, T carrier, Func<T, string, IEnumerable<string>> getter)
+ public override PropagationContext Extract<T>(PropagationContext context, T carrier, Func<T, string, IEnumerable<string>?> getter)
{
if (context.ActivityContext.IsValid())
{
diff --git a/test/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule.Tests/ActivityHelperTest.cs b/test/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule.Tests/ActivityHelperTest.cs
index 0e7bacce..d5dabf7c 100644
--- a/test/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule.Tests/ActivityHelperTest.cs
+++ b/test/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule.Tests/ActivityHelperTest.cs
@@ -555,7 +555,7 @@ public class ActivityHelperTest : IDisposable
{
public override ISet<string>? Fields => null;
- public override PropagationContext Extract<T>(PropagationContext context, T carrier, Func<T, string, IEnumerable<string>> getter)
+ public override PropagationContext Extract<T>(PropagationContext context, T carrier, Func<T, string, IEnumerable<string>?> getter)
{
return default;
}
diff --git a/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/BasicTests.cs b/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/BasicTests.cs
index 03cc1600..4eeed567 100644
--- a/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/BasicTests.cs
+++ b/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/BasicTests.cs
@@ -1202,7 +1202,7 @@ public sealed class BasicTests
public override ISet<string> Fields => throw new NotImplementedException();
- public override PropagationContext Extract<T>(PropagationContext context, T carrier, Func<T, string, IEnumerable<string>> getter)
+ public override PropagationContext Extract<T>(PropagationContext context, T carrier, Func<T, string, IEnumerable<string>?> getter)
{
return new PropagationContext(this.activityContext, this.baggage);
}
diff --git a/test/OpenTelemetry.Instrumentation.GrpcCore.Tests/GrpcCoreClientInterceptorTests.cs b/test/OpenTelemetry.Instrumentation.GrpcCore.Tests/GrpcCoreClientInterceptorTests.cs
index 150f20a3..57db6c4d 100644
--- a/test/OpenTelemetry.Instrumentation.GrpcCore.Tests/GrpcCoreClientInterceptorTests.cs
+++ b/test/OpenTelemetry.Instrumentation.GrpcCore.Tests/GrpcCoreClientInterceptorTests.cs
@@ -389,9 +389,11 @@ public class GrpcCoreClientInterceptorTests
{
propagatorCalled++;
capturedPropagationContext = propagation;
- capturedCarrier = (Metadata)carrier;
+ capturedCarrier = (Metadata?)carrier;
// Make sure the original metadata make it through
+
+ Assert.NotNull(capturedCarrier);
if (additionalMetadata != null)
{
Assert.Equal(capturedCarrier, additionalMetadata);
diff --git a/test/Shared/CustomTextMapPropagator.cs b/test/Shared/CustomTextMapPropagator.cs
index 8d3404e8..fc869441 100644
--- a/test/Shared/CustomTextMapPropagator.cs
+++ b/test/Shared/CustomTextMapPropagator.cs
@@ -24,7 +24,7 @@ internal sealed class CustomTextMapPropagator : TextMapPropagator
#pragma warning restore SA1010 // Opening square brackets should be spaced correctly
#pragma warning restore SA1201 // Elements should appear in the correct order
- public override PropagationContext Extract<T>(PropagationContext context, T carrier, Func<T, string, IEnumerable<string>> getter)
+ public override PropagationContext Extract<T>(PropagationContext context, T carrier, Func<T, string, IEnumerable<string>?> getter)
{
if (this.TraceId != default && this.SpanId != default)
{
diff --git a/test/Shared/TestTextMapPropagator.cs b/test/Shared/TestTextMapPropagator.cs
index ba58606a..5c2c36da 100644
--- a/test/Shared/TestTextMapPropagator.cs
+++ b/test/Shared/TestTextMapPropagator.cs
@@ -9,13 +9,13 @@ namespace OpenTelemetry.Tests;
internal class TestTextMapPropagator : TextMapPropagator
{
- public Action<PropagationContext, object, Action<object, string, string>>? OnInject { get; set; }
+ public Action<PropagationContext, object?, Action<object, string, string>>? OnInject { get; set; }
public Action? Extracted { get; set; }
public override ISet<string> Fields => throw new NotImplementedException();
- public override PropagationContext Extract<T>(PropagationContext context, T carrier, Func<T, string, IEnumerable<string>> getter)
+ public override PropagationContext Extract<T>(PropagationContext context, T carrier, Func<T, string, IEnumerable<string>?> getter)
{
this.Extracted?.Invoke();
return context;
@@ -23,10 +23,9 @@ internal class TestTextMapPropagator : TextMapPropagator
public override void Inject<T>(PropagationContext context, T carrier, Action<T, string, string> setter)
{
- var newAction = new Action<T, string, string>((c, k, v) => setter(c, k, v));
this.OnInject?.Invoke(
context,
carrier,
- new Action<object, string, string>((c, k, v) => setter((T)c, k, v)));
+ (c, k, v) => setter((T)c, k, v));
}
} |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #5767 +/- ##
==========================================
+ Coverage 83.38% 86.21% +2.82%
==========================================
Files 297 256 -41
Lines 12531 11147 -1384
==========================================
- Hits 10449 9610 -839
+ Misses 2082 1537 -545
Flags with carried forward coverage won't be shown. Click here to find out more.
|
CodeBlanch
reviewed
Aug 1, 2024
| @@ -102,11 +104,11 @@ public override void Inject<T>(PropagationContext context, T carrier, Action<T, | |||
| } | |||
| } | |||
|
|
|||
| internal static bool TryExtractBaggage(string[] baggageCollection, out Dictionary<string, string> baggage) | |||
| internal static bool TryExtractBaggage(string[] baggageCollection, out Dictionary<string, string>? baggage) | |||
There was a problem hiding this comment.
Suggested change
| internal static bool TryExtractBaggage(string[] baggageCollection, out Dictionary<string, string>? baggage) | |
| internal static bool TryExtractBaggage(string[] baggageCollection, [NotNullWhen(true)] out Dictionary<string, string>? baggage) |
Might also need tweaks to usings.
There was a problem hiding this comment.
@CodeBlanch, It is not so simple. Nullability shims are not included in OpenTelemetry.Api. It is not possible due to InternalsVisibleTo("OpenTelemetry") project.
When adding this shims, 2 attributes with exactly same names occurs in OpenTelemetry netstandard2.1 project.
To solve this, we could add additional target for OpenTelemetry.Api (netstandard2.1).
Fixes without adding shims in 8bb5e43
|
This PR was marked stale due to lack of activity and will be closed in 7 days. Commenting or Pushing will instruct the bot to automatically remove the label. This bot runs once per day. |
github-actions
bot
added
Stale
rajkumar-rangaraj
approved these changes
Aug 14, 2024
|
This PR was marked stale due to lack of activity and will be closed in 7 days. Commenting or pushing will instruct the bot to automatically remove the label. This bot runs once per day. |
github-actions
bot
added
Stale
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Towards #3958
Changes
[Propagators] Nullable
Merge requirement checklist
[ ] AppropriateCHANGELOG.mdfiles updated for non-trivial changes