Merge pull request #1515 from DuendeSoftware/anders/otel-fixes

Open Telemetry Fixes
DuendeSoftware · Feb 9, 2024 · e130e8b · e130e8b
2 parents 2f7360c + 8867d5d
commit e130e8b
Show file tree

Hide file tree

Showing 4 changed files with 142 additions and 374 deletions.
diff --git a/hosts/main/HostingExtensions.cs b/hosts/main/HostingExtensions.cs
@@ -9,6 +9,7 @@
 using Microsoft.IdentityModel.Tokens;
 using OpenTelemetry.Metrics;
 using OpenTelemetry.Resources;
+using OpenTelemetry.Trace;
 using Serilog;
 using Serilog.Events;
 
@@ -41,15 +42,24 @@ internal static WebApplication ConfigureServices(this WebApplicationBuilder buil
             return Task.FromResult(principal);
         });
 
-        var openTelemetry = builder.Services.AddOpenTelemetry();
+var openTelemetry = builder.Services.AddOpenTelemetry();
 
-        openTelemetry.ConfigureResource(r => r
-            .AddService(builder.Environment.ApplicationName));
+openTelemetry.ConfigureResource(r => r
+    .AddService(builder.Environment.ApplicationName));
 
-        openTelemetry.WithMetrics(m => m
-            .AddMeter(Telemetry.ServiceName)
-            .AddMeter(Pages.Telemetry.ServiceName)
-            .AddPrometheusExporter()) ;
+openTelemetry.WithMetrics(m => m
+    .AddMeter(Telemetry.ServiceName)
+    .AddMeter(Pages.Telemetry.ServiceName)
+    .AddPrometheusExporter());
+
+openTelemetry.WithTracing(t => t
+    .AddSource(IdentityServerConstants.Tracing.Basic)
+    .AddSource(IdentityServerConstants.Tracing.Cache)
+    .AddSource(IdentityServerConstants.Tracing.Services)
+    .AddSource(IdentityServerConstants.Tracing.Stores)
+    .AddSource(IdentityServerConstants.Tracing.Validation)
+    .AddAspNetCoreInstrumentation()
+    .AddConsoleExporter());
 
         return builder.Build();
     }

diff --git a/hosts/main/Pages/Telemetry.cs b/hosts/main/Pages/Telemetry.cs
@@ -25,97 +25,70 @@ public static class Telemetry
     /// </summary>
     public static class Metrics
     {
+#pragma warning disable 1591
+
         /// <summary>
         /// Name of Counters
         /// </summary>
         public static class Counters
         {
-            /// <summary>
-            /// consent_granted
-            /// </summary>
-            public const string ConsentGranted = "consent_granted";
-
-            /// <summary>
-            /// consent_denied
-            /// </summary>
-            public const string ConsentDenied = "consent_denied";
-
-            /// <summary>
-            /// grants_revoked
-            /// </summary>
-            public const string GrantsRevoked = "grants_revoked";
-
-            /// <summary>
-            /// user_login
-            /// </summary>
-            public const string UserLogin = "user_login";
-
-            /// <summary>
-            /// user_login_failure
-            /// </summary>
-            public const string UserLoginFailure = "user_login_failure";
-
-            /// <summary>
-            /// user_logout
-            /// </summary>
-            public const string UserLogout = "user_logout";
+            public const string Consent = "tokenservice.consent";
+            public const string GrantsRevoked = "tokenservice.grants_revoked";
+            public const string UserLogin = "tokenservice.user_login";
+            public const string UserLogout = "tokenservice.user_logout";
         }
 
         /// <summary>
         /// Name of tags
         /// </summary>
         public static class Tags
         {
-            /// <summary>
-            /// client
-            /// </summary>
             public const string Client = "client";
-
-            /// <summary>
-            /// error
-            /// </summary>
             public const string Error = "error";
-
-            /// <summary>
-            /// idp
-            /// </summary>
             public const string Idp = "idp";
-
-            /// <summary>
-            /// remember
-            /// </summary>
             public const string Remember = "remember";
-
-            /// <summary>
-            /// scope
-            /// </summary>
             public const string Scope = "scope";
+            public const string Consent = "consent";
         }
 
+        /// <summary>
+        /// Values of tags
+        /// </summary>
+        public static class TagValues
+        {
+            public const string Granted = "granted";
+            public const string Denied = "denied";
+        }
+
+#pragma warning restore 1591
+
         /// <summary>
         /// Meter for the IdentityServer host project
         /// </summary>
         private static readonly Meter Meter = new Meter(ServiceName, ServiceVersion);
 
-        private static Counter<long> ConsentGrantedCounter = Meter.CreateCounter<long>(Counters.ConsentGranted);
+        private static Counter<long> ConsentCounter = Meter.CreateCounter<long>(Counters.Consent);
 
         /// <summary>
-        /// Helper method to increase <see cref="Counters.ConsentGranted"/> counter. The scopes
+        /// Helper method to increase <see cref="Counters.Consent"/> counter. The scopes
         /// are expanded and called one by one to not cause a combinatory explosion of scopes.
         /// </summary>
         /// <param name="clientId">Client id</param>
         /// <param name="scopes">Scope names. Each element is added on it's own to the counter</param>
         public static void ConsentGranted(string clientId, IEnumerable<string> scopes, bool remember)
         {
             ArgumentNullException.ThrowIfNull(scopes);
-            foreach(var scope in scopes)
+
+            foreach (var scope in scopes)
             {
-                ConsentGrantedCounter.Add(1, new(Tags.Client, clientId), new(Tags.Scope, scope), new(Tags.Remember, remember));
+                ConsentCounter.Add(1,
+                    new(Tags.Client, clientId),
+                    new(Tags.Scope, scope),
+                    new(Tags.Remember, remember),
+                    new(Tags.Consent, TagValues.Granted));
             }
         }
 
-        private static Counter<long> ConsentDeniedCounter = Meter.CreateCounter<long>(Counters.ConsentDenied);
-
         /// <summary>
         /// Helper method to increase <see cref="Counters.ConsentDenied"/> counter. The scopes
         /// are expanded and called one by one to not cause a combinatory explosion of scopes.
@@ -127,7 +100,7 @@ public static void ConsentDenied(string clientId, IEnumerable<string> scopes)
             ArgumentNullException.ThrowIfNull(scopes);
             foreach (var scope in scopes)
             {
-                ConsentDeniedCounter.Add(1, new(Tags.Client, clientId), new(Tags.Scope, scope));
+                ConsentCounter.Add(1, new(Tags.Client, clientId), new(Tags.Scope, scope), new(Tags.Consent, TagValues.Denied));
             }
         }
 
@@ -149,15 +122,13 @@ public static void GrantsRevoked(string? clientId)
         public static void UserLogin(string? clientId, string idp)
             => UserLoginCounter.Add(1, new(Tags.Client, clientId), new(Tags.Idp, idp));
 
-        private static Counter<long> UserLoginFailureCounter = Meter.CreateCounter<long>(Counters.UserLoginFailure);
-
         /// <summary>
-        /// Helper method to increase <see cref="Counters.UserLoginFailure" counter.
+        /// Helper method to increase <see cref="Counters.UserLogin" counter on failure.
         /// </summary>
         /// <param name="clientId">Client Id, if available</param>
         /// <param name="error">Error message</param>
         public static void UserLoginFailure(string? clientId, string idp, string error)
-            => UserLoginFailureCounter.Add(1, new(Tags.Client, clientId), new(Tags.Idp, idp), new(Tags.Error, error));
+            => UserLoginCounter.Add(1, new(Tags.Client, clientId), new(Tags.Idp, idp), new(Tags.Error, error));
 
         private static Counter<long> UserLogoutCounter = Meter.CreateCounter<long>(Counters.UserLogout);
 

diff --git a/src/IdentityServer/Endpoints/BackchannelAuthenticationEndpoint.cs b/src/IdentityServer/Endpoints/BackchannelAuthenticationEndpoint.cs
@@ -76,8 +76,8 @@ private async Task<IEndpointResult> ProcessAuthenticationRequestAsync(HttpContex
         if (clientResult.IsError)
         {
             var error = clientResult.Error ?? OidcConstants.BackchannelAuthenticationRequestErrors.InvalidClient;
-            Telemetry.Metrics.BackChannelAuthenticationFailureCounter
-                .Add(1, new("client", clientResult.Client?.ClientId), new("error", error));
+            Telemetry.Metrics.BackChannelAuthenticationFailure(
+                clientResult.Client?.ClientId, error);
             return Error(error);
         }