Merge branch 'user/kcoulombe/HandleMeterDisposal' of https://github.c…

…om/stonkie/opentelemetry-dotnet into user/kcoulombe/HandleMeterDisposal
open-telemetry · Nov 29, 2024 · 57bf5d0 · 57bf5d0
2 parents 8ab84af + 60aa3db
commit 57bf5d0
Show file tree

Hide file tree

Showing 24 changed files with 475 additions and 135 deletions.
diff --git a/...ry.Exporter.OpenTelemetryProtocol/Implementation/ExportClient/BaseOtlpGrpcExportClient.cs b/...ry.Exporter.OpenTelemetryProtocol/Implementation/ExportClient/BaseOtlpGrpcExportClient.cs
@@ -13,7 +13,13 @@ namespace OpenTelemetry.Exporter.OpenTelemetryProtocol.Implementation.ExportClie
 /// <typeparam name="TRequest">Type of export request.</typeparam>
 internal abstract class BaseOtlpGrpcExportClient<TRequest> : IExportClient<TRequest>
 {
-    protected static readonly ExportClientGrpcResponse SuccessExportResponse = new ExportClientGrpcResponse(success: true, deadlineUtc: default, exception: null);
+    protected static readonly ExportClientGrpcResponse SuccessExportResponse
+        = new(
+            success: true,
+            deadlineUtc: default,
+            exception: null,
+            status: null,
+            grpcStatusDetailsHeader: null);
 
     protected BaseOtlpGrpcExportClient(OtlpExporterOptions options)
     {

diff --git a/...ry.Exporter.OpenTelemetryProtocol/Implementation/ExportClient/ExportClientGrpcResponse.cs b/...ry.Exporter.OpenTelemetryProtocol/Implementation/ExportClient/ExportClientGrpcResponse.cs
@@ -1,15 +1,25 @@
 // Copyright The OpenTelemetry Authors
 // SPDX-License-Identifier: Apache-2.0
 
+using OpenTelemetry.Exporter.OpenTelemetryProtocol.Implementation.ExportClient.Grpc;
+
 namespace OpenTelemetry.Exporter.OpenTelemetryProtocol.Implementation.ExportClient;
 
 internal sealed class ExportClientGrpcResponse : ExportClientResponse
 {
     public ExportClientGrpcResponse(
         bool success,
         DateTime deadlineUtc,
-        Exception? exception)
+        Exception? exception,
+        Status? status,
+        string? grpcStatusDetailsHeader)
         : base(success, deadlineUtc, exception)
     {
+        this.Status = status;
+        this.GrpcStatusDetailsHeader = grpcStatusDetailsHeader;
     }
+
+    public Status? Status { get; }
+
+    public string? GrpcStatusDetailsHeader { get; }
 }
diff --git a/...emetry.Exporter.OpenTelemetryProtocol/Implementation/ExportClient/ExportClientResponse.cs b/...emetry.Exporter.OpenTelemetryProtocol/Implementation/ExportClient/ExportClientResponse.cs
@@ -1,8 +1,6 @@
 // Copyright The OpenTelemetry Authors
 // SPDX-License-Identifier: Apache-2.0
 
-using System.Diagnostics.CodeAnalysis;
-
 namespace OpenTelemetry.Exporter.OpenTelemetryProtocol.Implementation.ExportClient;
 
 internal abstract class ExportClientResponse
@@ -14,7 +12,6 @@ protected ExportClientResponse(bool success, DateTime deadlineUtc, Exception? ex
         this.DeadlineUtc = deadlineUtc;
     }
 
-    [MemberNotNullWhen(false, nameof(Exception))]
     public bool Success { get; }
 
     public Exception? Exception { get; }

diff --git a/...ry.Exporter.OpenTelemetryProtocol/Implementation/ExportClient/Grpc/GrpcProtocolHelpers.cs b/...ry.Exporter.OpenTelemetryProtocol/Implementation/ExportClient/Grpc/GrpcProtocolHelpers.cs
@@ -15,7 +15,6 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-using System.Diagnostics.CodeAnalysis;
 #if NET462
 using System.Net.Http;
 #endif
@@ -27,38 +26,30 @@ internal static class GrpcProtocolHelpers
 {
     internal const string StatusTrailer = "grpc-status";
     internal const string MessageTrailer = "grpc-message";
-    internal const string CancelledDetail = "No grpc-status found on response.";
 
-    public static Status? GetResponseStatus(HttpHeaders trailingHeaders, HttpResponseMessage httpResponse)
+    public static Status GetResponseStatus(HttpResponseMessage httpResponse, HttpHeaders trailingHeaders)
     {
-        Status? status;
         try
         {
-            var result = trailingHeaders.Any() ? TryGetStatusCore(trailingHeaders, out status) : TryGetStatusCore(httpResponse.Headers, out status);
-
-            if (!result)
-            {
-                status = new Status(StatusCode.Cancelled, CancelledDetail);
-            }
+            return trailingHeaders.Any()
+                ? GetStatusCore(trailingHeaders)
+                : GetStatusCore(httpResponse.Headers);
         }
         catch (Exception ex)
         {
             // Handle error from parsing badly formed status
-            status = new Status(StatusCode.Cancelled, ex.Message, ex);
+            return new Status(StatusCode.Internal, ex.Message, ex);
         }
-
-        return status;
     }
 
-    public static bool TryGetStatusCore(HttpHeaders headers, [NotNullWhen(true)] out Status? status)
+    public static Status GetStatusCore(HttpHeaders headers)
     {
         var grpcStatus = GetHeaderValue(headers, StatusTrailer);
 
         // grpc-status is a required trailer
         if (grpcStatus == null)
         {
-            status = null;
-            return false;
+            return Status.NoReply;
         }
 
         int statusValue;
@@ -79,8 +70,7 @@ public static bool TryGetStatusCore(HttpHeaders headers, [NotNullWhen(true)] out
             grpcMessage = Uri.UnescapeDataString(grpcMessage);
         }
 
-        status = new Status((StatusCode)statusValue, grpcMessage ?? string.Empty);
-        return true;
+        return new Status((StatusCode)statusValue, grpcMessage ?? string.Empty);
     }
 
     public static string? GetHeaderValue(HttpHeaders? headers, string name, bool first = false)

diff --git a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/ExportClient/Grpc/Status.cs b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/ExportClient/Grpc/Status.cs
@@ -25,6 +25,8 @@ namespace OpenTelemetry.Exporter.OpenTelemetryProtocol.Implementation.ExportClie
 [DebuggerDisplay("{DebuggerToString(),nq}")]
 internal struct Status
 {
+    public const string NoReplyDetailMessage = "No grpc-status found on response.";
+
     /// <summary>
     /// Default result of a successful RPC. StatusCode=OK, empty details message.
     /// </summary>
@@ -35,6 +37,11 @@ internal struct Status
     /// </summary>
     public static readonly Status DefaultCancelled = new Status(StatusCode.Cancelled, string.Empty);
 
+    /// <summary>
+    /// Default result of a cancelled RPC with no grpc-status found on response.
+    /// </summary>
+    public static readonly Status NoReply = new Status(StatusCode.Internal, NoReplyDetailMessage);
+
     /// <summary>
     /// Initializes a new instance of the <see cref="Status"/> struct.
     /// </summary>

diff --git a/...try.Exporter.OpenTelemetryProtocol/Implementation/ExportClient/OtlpGrpcLogExportClient.cs b/...try.Exporter.OpenTelemetryProtocol/Implementation/ExportClient/OtlpGrpcLogExportClient.cs
@@ -39,7 +39,7 @@ public override ExportClientResponse SendExportRequest(OtlpCollector.ExportLogsS
         {
             OpenTelemetryProtocolExporterEventSource.Log.FailedToReachCollector(this.Endpoint, ex);
 
-            return new ExportClientGrpcResponse(success: false, deadlineUtc: deadlineUtc, exception: ex);
+            return new ExportClientGrpcResponse(success: false, deadlineUtc, ex, null, null);
         }
     }
 }
diff --git a/...Exporter.OpenTelemetryProtocol/Implementation/ExportClient/OtlpGrpcMetricsExportClient.cs b/...Exporter.OpenTelemetryProtocol/Implementation/ExportClient/OtlpGrpcMetricsExportClient.cs
@@ -39,7 +39,7 @@ public override ExportClientResponse SendExportRequest(OtlpCollector.ExportMetri
         {
             OpenTelemetryProtocolExporterEventSource.Log.FailedToReachCollector(this.Endpoint, ex);
 
-            return new ExportClientGrpcResponse(success: false, deadlineUtc: deadlineUtc, exception: ex);
+            return new ExportClientGrpcResponse(false, deadlineUtc, ex, null, null);
         }
     }
 }
diff --git a/...y.Exporter.OpenTelemetryProtocol/Implementation/ExportClient/OtlpGrpcTraceExportClient.cs b/...y.Exporter.OpenTelemetryProtocol/Implementation/ExportClient/OtlpGrpcTraceExportClient.cs
@@ -39,7 +39,7 @@ public override ExportClientResponse SendExportRequest(OtlpCollector.ExportTrace
         {
             OpenTelemetryProtocolExporterEventSource.Log.FailedToReachCollector(this.Endpoint, ex);
 
-            return new ExportClientGrpcResponse(success: false, deadlineUtc: deadlineUtc, exception: ex);
+            return new ExportClientGrpcResponse(false, deadlineUtc, ex, null, null);
         }
     }
 }
diff --git a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/ExportClient/OtlpRetry.cs b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/ExportClient/OtlpRetry.cs
@@ -83,12 +83,54 @@ public static bool ShouldHandleHttpRequestException(Exception? exception)
 
     public static bool TryGetGrpcRetryResult(ExportClientGrpcResponse response, int retryDelayMilliseconds, out RetryResult retryResult)
     {
+        retryResult = default;
+
         if (response.Exception is RpcException rpcException)
         {
             return TryGetRetryResult(rpcException.StatusCode, IsGrpcStatusCodeRetryable, response.DeadlineUtc, rpcException.Trailers, TryGetGrpcRetryDelay, retryDelayMilliseconds, out retryResult);
         }
+        else if (response.Status != null)
+        {
+            var nextRetryDelayMilliseconds = retryDelayMilliseconds;
+
+            if (IsDeadlineExceeded(response.DeadlineUtc))
+            {
+                return false;
+            }
+
+            var throttleDelay = Grpc.GrpcStatusDeserializer.TryGetGrpcRetryDelay(response.GrpcStatusDetailsHeader);
+            var retryable = IsGrpcStatusCodeRetryable(response.Status.Value.StatusCode, throttleDelay.HasValue);
+
+            if (!retryable)
+            {
+                return false;
+            }
+
+            var delayDuration = throttleDelay ?? TimeSpan.FromMilliseconds(GetRandomNumber(0, nextRetryDelayMilliseconds));
+
+            if (IsDeadlineExceeded(response.DeadlineUtc + delayDuration))
+            {
+                return false;
+            }
+
+            if (throttleDelay.HasValue)
+            {
+                try
+                {
+                    // TODO: Consider making nextRetryDelayMilliseconds a double to avoid the need for convert/overflow handling
+                    nextRetryDelayMilliseconds = Convert.ToInt32(throttleDelay.Value.TotalMilliseconds);
+                }
+                catch (OverflowException)
+                {
+                    nextRetryDelayMilliseconds = MaxBackoffMilliseconds;
+                }
+            }
+
+            nextRetryDelayMilliseconds = CalculateNextRetryDelay(nextRetryDelayMilliseconds);
+            retryResult = new RetryResult(throttleDelay.HasValue, delayDuration, nextRetryDelayMilliseconds);
+            return true;
+        }
 
-        retryResult = default;
         return false;
     }
 
@@ -216,6 +258,24 @@ private static bool IsGrpcStatusCodeRetryable(StatusCode statusCode, bool hasRet
         }
     }
 
+    private static bool IsGrpcStatusCodeRetryable(Grpc.StatusCode statusCode, bool hasRetryDelay)
+    {
+        switch (statusCode)
+        {
+            case Grpc.StatusCode.Cancelled:
+            case Grpc.StatusCode.DeadlineExceeded:
+            case Grpc.StatusCode.Aborted:
+            case Grpc.StatusCode.OutOfRange:
+            case Grpc.StatusCode.Unavailable:
+            case Grpc.StatusCode.DataLoss:
+                return true;
+            case Grpc.StatusCode.ResourceExhausted:
+                return hasRetryDelay;
+            default:
+                return false;
+        }
+    }
+
     private static bool IsHttpStatusCodeRetryable(HttpStatusCode statusCode, bool hasRetryDelay)
     {
         switch (statusCode)