From 8a22eaf81e82e1a11d1fd35500a8555a26afe530 Mon Sep 17 00:00:00 2001 From: Cijo Thomas Date: Wed, 22 Apr 2020 17:14:14 -0700 Subject: [PATCH] Rename CanonicalCode to StatusCanonicalCode (#618) * Add new ctor to Status, Rename CanonicalCode to StatusCanonicalCode. Both changes make code match the spec. * revert Status public ctor. There already exists a more efficient way to create Status without ctor. * minor * PropertyName in Status to be simply CanonicalCode ionstead of StatusCanonicalCode, as the name of class itself is Status already and hence no need to repeat in property name Co-authored-by: Sergey Kanzhelev --- src/OpenTelemetry.Api/Trace/Status.cs | 42 +++++++++---------- ...anonicalCode.cs => StatusCanonicalCode.cs} | 8 +++- .../JaegerConversionExtensions.cs | 2 +- .../ZipkinConversionExtensions.cs | 2 +- .../HttpInListenerTests.cs | 2 +- .../HttpClientTests.netcore31.cs | 36 ++++++++-------- .../HttpWebRequestTests.net461.cs | 36 ++++++++-------- .../SqlClientTests.cs | 4 +- .../Impl/Trace/StatusTest.cs | 24 +++++------ .../Impl/Utils/CanonicalCodeExtensions.cs | 2 +- 10 files changed, 81 insertions(+), 77 deletions(-) rename src/OpenTelemetry.Api/Trace/{CanonicalCode.cs => StatusCanonicalCode.cs} (95%) diff --git a/src/OpenTelemetry.Api/Trace/Status.cs b/src/OpenTelemetry.Api/Trace/Status.cs index a1023403e..97e8e3655 100644 --- a/src/OpenTelemetry.Api/Trace/Status.cs +++ b/src/OpenTelemetry.Api/Trace/Status.cs @@ -24,12 +24,12 @@ namespace OpenTelemetry.Trace /// /// The operation completed successfully. /// - public static readonly Status Ok = new Status(CanonicalCode.Ok); + public static readonly Status Ok = new Status(StatusCanonicalCode.Ok); /// /// The operation was cancelled (typically by the caller). /// - public static readonly Status Cancelled = new Status(CanonicalCode.Cancelled); + public static readonly Status Cancelled = new Status(StatusCanonicalCode.Cancelled); /// /// Unknown error. An example of where this error may be returned is if a Status value received @@ -37,14 +37,14 @@ namespace OpenTelemetry.Trace /// Also errors raised by APIs that do not return enough error information may be converted to /// this error. /// - public static readonly Status Unknown = new Status(CanonicalCode.Unknown); + public static readonly Status Unknown = new Status(StatusCanonicalCode.Unknown); /// /// Client specified an invalid argument. Note that this differs from FAILED_PRECONDITION. /// INVALID_ARGUMENT indicates arguments that are problematic regardless of the state of the /// system (e.g., a malformed file name). /// - public static readonly Status InvalidArgument = new Status(CanonicalCode.InvalidArgument); + public static readonly Status InvalidArgument = new Status(StatusCanonicalCode.InvalidArgument); /// /// Deadline expired before operation could complete. For operations that change the state of the @@ -52,17 +52,17 @@ namespace OpenTelemetry.Trace /// example, a successful response from a server could have been delayed long enough for the /// deadline to expire. /// - public static readonly Status DeadlineExceeded = new Status(CanonicalCode.DeadlineExceeded); + public static readonly Status DeadlineExceeded = new Status(StatusCanonicalCode.DeadlineExceeded); /// /// Some requested entity (e.g., file or directory) was not found. /// - public static readonly Status NotFound = new Status(CanonicalCode.NotFound); + public static readonly Status NotFound = new Status(StatusCanonicalCode.NotFound); /// /// Some entity that we attempted to create (e.g., file or directory) already exists. /// - public static readonly Status AlreadyExists = new Status(CanonicalCode.AlreadyExists); + public static readonly Status AlreadyExists = new Status(StatusCanonicalCode.AlreadyExists); /// /// The caller does not have permission to execute the specified operation. PERMISSION_DENIED @@ -70,18 +70,18 @@ namespace OpenTelemetry.Trace /// instead for those errors). PERMISSION_DENIED must not be used if the caller cannot be /// identified (use UNAUTHENTICATED instead for those errors). /// - public static readonly Status PermissionDenied = new Status(CanonicalCode.PermissionDenied); + public static readonly Status PermissionDenied = new Status(StatusCanonicalCode.PermissionDenied); /// /// The request does not have valid authentication credentials for the operation. /// - public static readonly Status Unauthenticated = new Status(CanonicalCode.Unauthenticated); + public static readonly Status Unauthenticated = new Status(StatusCanonicalCode.Unauthenticated); /// /// Some resource has been exhausted, perhaps a per-user quota, or perhaps the entire file system /// is out of space. /// - public static readonly Status ResourceExhausted = new Status(CanonicalCode.ResourceExhausted); + public static readonly Status ResourceExhausted = new Status(StatusCanonicalCode.ResourceExhausted); /// /// Operation was rejected because the system is not in a state required for the operation's @@ -95,13 +95,13 @@ namespace OpenTelemetry.Trace /// is non-empty, FAILED_PRECONDITION should be returned since the client should not retry unless /// they have first fixed up the directory by deleting files from it. /// - public static readonly Status FailedPrecondition = new Status(CanonicalCode.FailedPrecondition); + public static readonly Status FailedPrecondition = new Status(StatusCanonicalCode.FailedPrecondition); /// /// The operation was aborted, typically due to a concurrency issue like sequencer check /// failures, transaction aborts, etc. /// - public static readonly Status Aborted = new Status(CanonicalCode.Aborted); + public static readonly Status Aborted = new Status(StatusCanonicalCode.Aborted); /// /// Operation was attempted past the valid range. E.g., seeking or reading past end of file. @@ -116,18 +116,18 @@ namespace OpenTelemetry.Trace /// iterating through a space can easily look for an OUT_OF_RANGE error to detect when they are /// done. /// - public static readonly Status OutOfRange = new Status(CanonicalCode.OutOfRange); + public static readonly Status OutOfRange = new Status(StatusCanonicalCode.OutOfRange); /// /// Operation is not implemented or not supported/enabled in this service. /// - public static readonly Status Unimplemented = new Status(CanonicalCode.Unimplemented); + public static readonly Status Unimplemented = new Status(StatusCanonicalCode.Unimplemented); /// /// Internal errors. Means some invariants expected by underlying system has been broken. If you /// see one of these errors, something is very broken. /// - public static readonly Status Internal = new Status(CanonicalCode.Internal); + public static readonly Status Internal = new Status(StatusCanonicalCode.Internal); /// /// The service is currently unavailable. This is a most likely a transient condition and may be @@ -135,16 +135,16 @@ namespace OpenTelemetry.Trace /// /// See litmus test above for deciding between FAILED_PRECONDITION, ABORTED, and UNAVAILABLE. /// - public static readonly Status Unavailable = new Status(CanonicalCode.Unavailable); + public static readonly Status Unavailable = new Status(StatusCanonicalCode.Unavailable); /// /// Unrecoverable data loss or corruption. /// - public static readonly Status DataLoss = new Status(CanonicalCode.DataLoss); + public static readonly Status DataLoss = new Status(StatusCanonicalCode.DataLoss); - internal Status(CanonicalCode canonicalCode, string description = null) + internal Status(StatusCanonicalCode statusCanonicalCode, string description = null) { - this.CanonicalCode = canonicalCode; + this.CanonicalCode = statusCanonicalCode; this.Description = description; this.IsValid = true; } @@ -158,7 +158,7 @@ namespace OpenTelemetry.Trace /// /// Gets the canonical code from this status. /// - public CanonicalCode CanonicalCode { get; } + public StatusCanonicalCode CanonicalCode { get; } /// /// Gets the status description. @@ -168,7 +168,7 @@ namespace OpenTelemetry.Trace /// /// Gets a value indicating whether span completed sucessfully. /// - public bool IsOk => this.CanonicalCode == CanonicalCode.Ok; + public bool IsOk => this.CanonicalCode == StatusCanonicalCode.Ok; /// /// Compare two for equality. diff --git a/src/OpenTelemetry.Api/Trace/CanonicalCode.cs b/src/OpenTelemetry.Api/Trace/StatusCanonicalCode.cs similarity index 95% rename from src/OpenTelemetry.Api/Trace/CanonicalCode.cs rename to src/OpenTelemetry.Api/Trace/StatusCanonicalCode.cs index e5335d2f3..6e7b75f9c 100644 --- a/src/OpenTelemetry.Api/Trace/CanonicalCode.cs +++ b/src/OpenTelemetry.Api/Trace/StatusCanonicalCode.cs @@ -1,4 +1,4 @@ -// +// // Copyright 2018, OpenTelemetry Authors // // Licensed under the Apache License, Version 2.0 (the "License"); @@ -19,7 +19,11 @@ namespace OpenTelemetry.Trace /// /// Canonical result code of span execution. /// - public enum CanonicalCode + /// + /// This follows the standard GRPC codes. + /// https://github.com/grpc/grpc/blob/master/doc/statuscodes.md. + /// + public enum StatusCanonicalCode { /// /// The operation completed successfully. diff --git a/src/OpenTelemetry.Exporter.Jaeger/Implementation/JaegerConversionExtensions.cs b/src/OpenTelemetry.Exporter.Jaeger/Implementation/JaegerConversionExtensions.cs index 4c3ff6d5d..87cf4a753 100644 --- a/src/OpenTelemetry.Exporter.Jaeger/Implementation/JaegerConversionExtensions.cs +++ b/src/OpenTelemetry.Exporter.Jaeger/Implementation/JaegerConversionExtensions.cs @@ -56,7 +56,7 @@ namespace OpenTelemetry.Exporter.Jaeger.Implementation ["db.instance"] = 4, // peer.service for Redis. }; - private static readonly Dictionary CanonicalCodeDictionary = new Dictionary(); + private static readonly Dictionary CanonicalCodeDictionary = new Dictionary(); private static readonly DictionaryEnumerator.ForEachDelegate ProcessAttributeRef = ProcessAttribute; private static readonly DictionaryEnumerator.ForEachDelegate ProcessLibraryAttributeRef = ProcessLibraryAttribute; diff --git a/src/OpenTelemetry.Exporter.Zipkin/Implementation/ZipkinConversionExtensions.cs b/src/OpenTelemetry.Exporter.Zipkin/Implementation/ZipkinConversionExtensions.cs index 534cb0360..754b8fc7c 100644 --- a/src/OpenTelemetry.Exporter.Zipkin/Implementation/ZipkinConversionExtensions.cs +++ b/src/OpenTelemetry.Exporter.Zipkin/Implementation/ZipkinConversionExtensions.cs @@ -41,7 +41,7 @@ namespace OpenTelemetry.Exporter.Zipkin.Implementation private static readonly ConcurrentDictionary LocalEndpointCache = new ConcurrentDictionary(); private static readonly ConcurrentDictionary RemoteEndpointCache = new ConcurrentDictionary(); - private static readonly ConcurrentDictionary CanonicalCodeCache = new ConcurrentDictionary(); + private static readonly ConcurrentDictionary CanonicalCodeCache = new ConcurrentDictionary(); private static readonly DictionaryEnumerator.ForEachDelegate ProcessAttributesRef = ProcessAttributes; private static readonly DictionaryEnumerator.ForEachDelegate ProcessLibraryResourcesRef = ProcessLibraryResources; diff --git a/test/OpenTelemetry.Adapter.AspNet.Tests/HttpInListenerTests.cs b/test/OpenTelemetry.Adapter.AspNet.Tests/HttpInListenerTests.cs index 88fa68bfe..b1a159783 100644 --- a/test/OpenTelemetry.Adapter.AspNet.Tests/HttpInListenerTests.cs +++ b/test/OpenTelemetry.Adapter.AspNet.Tests/HttpInListenerTests.cs @@ -163,7 +163,7 @@ namespace OpenTelemetry.Adapter.AspNet.Tests Assert.Equal(routeTemplate ?? HttpContext.Current.Request.Path, span.Name); Assert.Equal(SpanKind.Server, span.Kind); - Assert.Equal(CanonicalCode.Ok, span.Status.CanonicalCode); + Assert.Equal(StatusCanonicalCode.Ok, span.Status.CanonicalCode); Assert.Equal("OK", span.Status.Description); var expectedUri = new Uri(url); diff --git a/test/OpenTelemetry.Adapter.Dependencies.Tests/HttpClientTests.netcore31.cs b/test/OpenTelemetry.Adapter.Dependencies.Tests/HttpClientTests.netcore31.cs index f360500b5..2eb204a3e 100644 --- a/test/OpenTelemetry.Adapter.Dependencies.Tests/HttpClientTests.netcore31.cs +++ b/test/OpenTelemetry.Adapter.Dependencies.Tests/HttpClientTests.netcore31.cs @@ -89,25 +89,25 @@ namespace OpenTelemetry.Adapter.Dependencies.Tests Assert.Equal(tc.SpanName, span.Name); Assert.Equal(tc.SpanKind, span.Kind.ToString()); - var d = new Dictionary() + var d = new Dictionary() { - { CanonicalCode.Ok, "OK"}, - { CanonicalCode.Cancelled, "CANCELLED"}, - { CanonicalCode.Unknown, "UNKNOWN"}, - { CanonicalCode.InvalidArgument, "INVALID_ARGUMENT"}, - { CanonicalCode.DeadlineExceeded, "DEADLINE_EXCEEDED"}, - { CanonicalCode.NotFound, "NOT_FOUND"}, - { CanonicalCode.AlreadyExists, "ALREADY_EXISTS"}, - { CanonicalCode.PermissionDenied, "PERMISSION_DENIED"}, - { CanonicalCode.ResourceExhausted, "RESOURCE_EXHAUSTED"}, - { CanonicalCode.FailedPrecondition, "FAILED_PRECONDITION"}, - { CanonicalCode.Aborted, "ABORTED"}, - { CanonicalCode.OutOfRange, "OUT_OF_RANGE"}, - { CanonicalCode.Unimplemented, "UNIMPLEMENTED"}, - { CanonicalCode.Internal, "INTERNAL"}, - { CanonicalCode.Unavailable, "UNAVAILABLE"}, - { CanonicalCode.DataLoss, "DATA_LOSS"}, - { CanonicalCode.Unauthenticated, "UNAUTHENTICATED"}, + { StatusCanonicalCode.Ok, "OK"}, + { StatusCanonicalCode.Cancelled, "CANCELLED"}, + { StatusCanonicalCode.Unknown, "UNKNOWN"}, + { StatusCanonicalCode.InvalidArgument, "INVALID_ARGUMENT"}, + { StatusCanonicalCode.DeadlineExceeded, "DEADLINE_EXCEEDED"}, + { StatusCanonicalCode.NotFound, "NOT_FOUND"}, + { StatusCanonicalCode.AlreadyExists, "ALREADY_EXISTS"}, + { StatusCanonicalCode.PermissionDenied, "PERMISSION_DENIED"}, + { StatusCanonicalCode.ResourceExhausted, "RESOURCE_EXHAUSTED"}, + { StatusCanonicalCode.FailedPrecondition, "FAILED_PRECONDITION"}, + { StatusCanonicalCode.Aborted, "ABORTED"}, + { StatusCanonicalCode.OutOfRange, "OUT_OF_RANGE"}, + { StatusCanonicalCode.Unimplemented, "UNIMPLEMENTED"}, + { StatusCanonicalCode.Internal, "INTERNAL"}, + { StatusCanonicalCode.Unavailable, "UNAVAILABLE"}, + { StatusCanonicalCode.DataLoss, "DATA_LOSS"}, + { StatusCanonicalCode.Unauthenticated, "UNAUTHENTICATED"}, }; Assert.Equal(tc.SpanStatus, d[span.Status.CanonicalCode]); diff --git a/test/OpenTelemetry.Adapter.Dependencies.Tests/HttpWebRequestTests.net461.cs b/test/OpenTelemetry.Adapter.Dependencies.Tests/HttpWebRequestTests.net461.cs index a4d364600..ec70c6acc 100644 --- a/test/OpenTelemetry.Adapter.Dependencies.Tests/HttpWebRequestTests.net461.cs +++ b/test/OpenTelemetry.Adapter.Dependencies.Tests/HttpWebRequestTests.net461.cs @@ -85,25 +85,25 @@ namespace OpenTelemetry.Adapter.Dependencies.Tests Assert.Equal(tc.SpanName, span.Name); Assert.Equal(tc.SpanKind, span.Kind.ToString()); - var d = new Dictionary() + var d = new Dictionary() { - { CanonicalCode.Ok, "OK"}, - { CanonicalCode.Cancelled, "CANCELLED"}, - { CanonicalCode.Unknown, "UNKNOWN"}, - { CanonicalCode.InvalidArgument, "INVALID_ARGUMENT"}, - { CanonicalCode.DeadlineExceeded, "DEADLINE_EXCEEDED"}, - { CanonicalCode.NotFound, "NOT_FOUND"}, - { CanonicalCode.AlreadyExists, "ALREADY_EXISTS"}, - { CanonicalCode.PermissionDenied, "PERMISSION_DENIED"}, - { CanonicalCode.ResourceExhausted, "RESOURCE_EXHAUSTED"}, - { CanonicalCode.FailedPrecondition, "FAILED_PRECONDITION"}, - { CanonicalCode.Aborted, "ABORTED"}, - { CanonicalCode.OutOfRange, "OUT_OF_RANGE"}, - { CanonicalCode.Unimplemented, "UNIMPLEMENTED"}, - { CanonicalCode.Internal, "INTERNAL"}, - { CanonicalCode.Unavailable, "UNAVAILABLE"}, - { CanonicalCode.DataLoss, "DATA_LOSS"}, - { CanonicalCode.Unauthenticated, "UNAUTHENTICATED"}, + { StatusCanonicalCode.Ok, "OK"}, + { StatusCanonicalCode.Cancelled, "CANCELLED"}, + { StatusCanonicalCode.Unknown, "UNKNOWN"}, + { StatusCanonicalCode.InvalidArgument, "INVALID_ARGUMENT"}, + { StatusCanonicalCode.DeadlineExceeded, "DEADLINE_EXCEEDED"}, + { StatusCanonicalCode.NotFound, "NOT_FOUND"}, + { StatusCanonicalCode.AlreadyExists, "ALREADY_EXISTS"}, + { StatusCanonicalCode.PermissionDenied, "PERMISSION_DENIED"}, + { StatusCanonicalCode.ResourceExhausted, "RESOURCE_EXHAUSTED"}, + { StatusCanonicalCode.FailedPrecondition, "FAILED_PRECONDITION"}, + { StatusCanonicalCode.Aborted, "ABORTED"}, + { StatusCanonicalCode.OutOfRange, "OUT_OF_RANGE"}, + { StatusCanonicalCode.Unimplemented, "UNIMPLEMENTED"}, + { StatusCanonicalCode.Internal, "INTERNAL"}, + { StatusCanonicalCode.Unavailable, "UNAVAILABLE"}, + { StatusCanonicalCode.DataLoss, "DATA_LOSS"}, + { StatusCanonicalCode.Unauthenticated, "UNAUTHENTICATED"}, }; Assert.Equal(tc.SpanStatus, d[span.Status.CanonicalCode]); diff --git a/test/OpenTelemetry.Adapter.Dependencies.Tests/SqlClientTests.cs b/test/OpenTelemetry.Adapter.Dependencies.Tests/SqlClientTests.cs index f01749d0d..57fd07740 100644 --- a/test/OpenTelemetry.Adapter.Dependencies.Tests/SqlClientTests.cs +++ b/test/OpenTelemetry.Adapter.Dependencies.Tests/SqlClientTests.cs @@ -108,7 +108,7 @@ namespace OpenTelemetry.Adapter.Dependencies.Tests Assert.Equal("master", span.Name); Assert.Equal(SpanKind.Client, span.Kind); - Assert.Equal(CanonicalCode.Ok, span.Status.CanonicalCode); + Assert.Equal(StatusCanonicalCode.Ok, span.Status.CanonicalCode); Assert.Null(span.Status.Description); Assert.Equal("sql", span.Attributes.FirstOrDefault(i => @@ -206,7 +206,7 @@ namespace OpenTelemetry.Adapter.Dependencies.Tests Assert.Equal("master", span.Name); Assert.Equal(SpanKind.Client, span.Kind); - Assert.Equal(CanonicalCode.Unknown, span.Status.CanonicalCode); + Assert.Equal(StatusCanonicalCode.Unknown, span.Status.CanonicalCode); Assert.Equal("Boom!", span.Status.Description); Assert.Equal("sql", span.Attributes.FirstOrDefault(i => diff --git a/test/OpenTelemetry.Tests/Impl/Trace/StatusTest.cs b/test/OpenTelemetry.Tests/Impl/Trace/StatusTest.cs index e59fec8d5..57391c3d9 100644 --- a/test/OpenTelemetry.Tests/Impl/Trace/StatusTest.cs +++ b/test/OpenTelemetry.Tests/Impl/Trace/StatusTest.cs @@ -22,7 +22,7 @@ namespace OpenTelemetry.Trace.Test [Fact] public void Status_Ok() { - Assert.Equal(CanonicalCode.Ok, Status.Ok.CanonicalCode); + Assert.Equal(StatusCanonicalCode.Ok, Status.Ok.CanonicalCode); Assert.Null(Status.Ok.Description); Assert.True(Status.Ok.IsOk); } @@ -31,7 +31,7 @@ namespace OpenTelemetry.Trace.Test public void CreateStatus_WithDescription() { var status = Status.Unknown.WithDescription("This is an error."); - Assert.Equal(CanonicalCode.Unknown, status.CanonicalCode); + Assert.Equal(StatusCanonicalCode.Unknown, status.CanonicalCode); Assert.Equal("This is an error.", status.Description); Assert.False(status.IsOk); } @@ -39,8 +39,8 @@ namespace OpenTelemetry.Trace.Test [Fact] public void Equality() { - var status1 = new Status(CanonicalCode.Ok); - var status2 = new Status(CanonicalCode.Ok); + var status1 = new Status(StatusCanonicalCode.Ok); + var status2 = new Status(StatusCanonicalCode.Ok); Assert.Equal(status1, status2); Assert.True(status1 == status2); @@ -49,8 +49,8 @@ namespace OpenTelemetry.Trace.Test [Fact] public void Equality_WithDescription() { - var status1 = new Status(CanonicalCode.Unknown, "error"); - var status2 = new Status(CanonicalCode.Unknown, "error"); + var status1 = new Status(StatusCanonicalCode.Unknown, "error"); + var status2 = new Status(StatusCanonicalCode.Unknown, "error"); Assert.Equal(status1, status2); Assert.True(status1 == status2); @@ -60,8 +60,8 @@ namespace OpenTelemetry.Trace.Test [Fact] public void Not_Equality() { - var status1 = new Status(CanonicalCode.Ok); - var status2 = new Status(CanonicalCode.Unknown); + var status1 = new Status(StatusCanonicalCode.Ok); + var status2 = new Status(StatusCanonicalCode.Unknown); Assert.NotEqual(status1, status2); Assert.True(status1 != status2); @@ -70,8 +70,8 @@ namespace OpenTelemetry.Trace.Test [Fact] public void Not_Equality_WithDescription1() { - var status1 = new Status(CanonicalCode.Ok, "ok"); - var status2 = new Status(CanonicalCode.Unknown, "error"); + var status1 = new Status(StatusCanonicalCode.Ok, "ok"); + var status2 = new Status(StatusCanonicalCode.Unknown, "error"); Assert.NotEqual(status1, status2); Assert.True(status1 != status2); @@ -81,8 +81,8 @@ namespace OpenTelemetry.Trace.Test [Fact] public void Not_Equality_WithDescription2() { - var status1 = new Status(CanonicalCode.Ok); - var status2 = new Status(CanonicalCode.Unknown, "error"); + var status1 = new Status(StatusCanonicalCode.Ok); + var status2 = new Status(StatusCanonicalCode.Unknown, "error"); Assert.NotEqual(status1, status2); Assert.True(status1 != status2); diff --git a/test/OpenTelemetry.Tests/Impl/Utils/CanonicalCodeExtensions.cs b/test/OpenTelemetry.Tests/Impl/Utils/CanonicalCodeExtensions.cs index 0e97d3359..0d2eb6f7a 100644 --- a/test/OpenTelemetry.Tests/Impl/Utils/CanonicalCodeExtensions.cs +++ b/test/OpenTelemetry.Tests/Impl/Utils/CanonicalCodeExtensions.cs @@ -19,7 +19,7 @@ namespace OpenTelemetry.Utils { internal static class CanonicalCodeExtensions { - public static Status ToStatus(this CanonicalCode code) + public static Status ToStatus(this StatusCanonicalCode code) { return new Status(code); }