Cloud Error Reporting Trace for ASP.NET Core #700

iantalarico · 2017-01-20T16:58:17Z

Allows for automated tracing of incoming requests to ASP.NET Core applications. Trace data is sent to the Stackdriver Trace API.

This PR includes:

Middleware for Tracing
Unit tests
Documentation
Snippets and examples

Coming soon:

Integration Tests
Delegate Handler for Tracing outgoing requests.

# Conflicts: # apis/Google.Cloud.Diagnostics.AspNet/Google.Cloud.Diagnostics.AspNet/Trace/Labels.cs

# Conflicts: # apis/Google.Cloud.Diagnostics.AspNet/Google.Cloud.Diagnostics.AspNet/Trace/CloudTrace.cs # apis/Google.Cloud.Diagnostics.AspNet/Google.Cloud.Diagnostics.AspNet/Trace/TraceHeaderContextUtils.cs # apis/Google.Cloud.Diagnostics.AspNet/Google.Cloud.Diagnostics.AspNet/Trace/TraceHeaderTraceOptionsFactory.cs # apis/Google.Cloud.Diagnostics.Common/Google.Cloud.Diagnostics.Common.Tests/AssemblyInfo.cs # apis/Google.Cloud.Diagnostics.Common/Google.Cloud.Diagnostics.Common.Tests/Trace/LabelsTest.cs # apis/Google.Cloud.Diagnostics.Common/Google.Cloud.Diagnostics.Common.Tests/Trace/SimpleManagedTracerTest.cs # apis/Google.Cloud.Diagnostics.Common/Google.Cloud.Diagnostics.Common/Trace/Labels.cs # apis/Google.Cloud.Diagnostics.Common/Google.Cloud.Diagnostics.Common/Trace/TraceHeaderContext.cs

...loud.Diagnostics.AspNetCore/Google.Cloud.Diagnostics.AspNetCore/Trace/CloudTraceExtension.cs

+        /// </summary>
+        /// <param name="projectId">The Google Cloud Platform project ID. Cannot be null.</param>
+        /// <param name="config">Optional trace configuration, if unset the default will be used.</param>
+        /// <param name="clientTask">Optional a task which produces the Trace client, if 


...loud.Diagnostics.AspNetCore/Google.Cloud.Diagnostics.AspNetCore/Trace/CloudTraceExtension.cs

+        {
+            // If the trace header says to trace or if the rate limiter allows tracing continue.
+            var headerContext = provider.GetService<TraceHeaderContext>();
+            if (!headerContext.ShouldTrace)


...oud.Diagnostics.AspNetCore/Google.Cloud.Diagnostics.AspNetCore/Trace/CloudTraceMiddleware.cs

+                // Trace the delegate and annotate it with information from the current
+                // http context.
+                tracer.StartSpan(httpContext.Request.Path);
+                await _next(httpContext);


...loud.Diagnostics.AspNetCore/Google.Cloud.Diagnostics.AspNetCore/Trace/CloudTraceExtension.cs

+        }
+
+        /// <summary>
+        /// Creates a <see cref="IManagedTracer"/> based on the <see cref="TraceHeaderContext"/> and


jskeet · 2017-01-20T17:38:53Z

As Mike seems to be on the case here, I'll wait until he's done - I wouldn't be reviewing it today anyway, as I'm off soon and this is a big thing to start last minute on Friday :)

iantalarico

Thanks Mike PTAL

...loud.Diagnostics.AspNetCore/Google.Cloud.Diagnostics.AspNetCore/Trace/CloudTraceExtension.cs

+        /// </summary>
+        /// <param name="projectId">The Google Cloud Platform project ID. Cannot be null.</param>
+        /// <param name="config">Optional trace configuration, if unset the default will be used.</param>
+        /// <param name="clientTask">Optional a task which produces the Trace client, if 


...loud.Diagnostics.AspNetCore/Google.Cloud.Diagnostics.AspNetCore/Trace/CloudTraceExtension.cs

+        }
+
+        /// <summary>
+        /// Creates a <see cref="IManagedTracer"/> based on the <see cref="TraceHeaderContext"/> and


...loud.Diagnostics.AspNetCore/Google.Cloud.Diagnostics.AspNetCore/Trace/CloudTraceExtension.cs

+        {
+            // If the trace header says to trace or if the rate limiter allows tracing continue.
+            var headerContext = provider.GetService<TraceHeaderContext>();
+            if (!headerContext.ShouldTrace)


...oud.Diagnostics.AspNetCore/Google.Cloud.Diagnostics.AspNetCore/Trace/CloudTraceMiddleware.cs

+                // Trace the delegate and annotate it with information from the current
+                // http context.
+                tracer.StartSpan(httpContext.Request.Path);
+                await _next(httpContext);


evildour

Just 2 additional comments. Everything else looks good.

...ud.Diagnostics.AspNetCore/Google.Cloud.Diagnostics.AspNetCore.Snippets/AspNetCoreSnippets.cs

+            // End sample
+
+            // Sample: UseTracer
+            /// <param name="tracer">Populated by dependency injection.</param>


...ostics.AspNetCore/Google.Cloud.Diagnostics.AspNetCore.Tests/Trace/CloudTraceExtensionTest.cs

+using System;
+using Xunit;
+
+using ProjectId =  Google.Cloud.Diagnostics.AspNetCore.CloudTraceExtension.ProjectId;


iantalarico · 2017-01-20T19:35:30Z

Thanks! PTAL

evildour

LGTM

jskeet

I haven't done a thorough review of the tests, but I love how easy it is to set up. Will see if I can use it from Azure easily, once I've honed my "getting Google credentials from Azure Key Vault" code...

...ostics.AspNetCore/Google.Cloud.Diagnostics.AspNetCore.Tests/Trace/CloudTraceExtensionTest.cs

+{
+    public class CloudTraceExtensionTest
+    {
+        private const string projectId = "pid";


...ostics.AspNetCore/Google.Cloud.Diagnostics.AspNetCore.Tests/Trace/CloudTraceExtensionTest.cs

+            TraceHeaderContext context, bool rateLimiterShouldTrace)
+        {
+            Mock<IConsumer<TraceProto>> mockConsumer = new Mock<IConsumer<TraceProto>>();
+            Mock<IServiceProvider> mockProvider = new Mock<IServiceProvider>();


...loud.Diagnostics.AspNetCore/Google.Cloud.Diagnostics.AspNetCore/Trace/CloudTraceExtension.cs

+    ///  finely grained manual tracing.
+    /// </summary>
+    /// 
+    /// <example>


...loud.Diagnostics.AspNetCore/Google.Cloud.Diagnostics.AspNetCore/Trace/CloudTraceExtension.cs

+            var traceIdFactory = provider.GetService<TraceIdFactory>();
+            var consumer = provider.GetService<IConsumer<TraceProto>>();
+
+            // Check that we have all the needed services.


...loud.Diagnostics.AspNetCore/Google.Cloud.Diagnostics.AspNetCore/Trace/CloudTraceExtension.cs

+                throw new InvalidOperationException("Ensure Google Cloud Trace is properly set up.");
+            }
+
+            // If the trace header says to trace or if the rate limiter allows tracing continue.


...loud.Diagnostics.AspNetCore/Google.Cloud.Diagnostics.AspNetCore/Trace/CloudTraceExtension.cs

+            // If the trace header says to trace or if the rate limiter allows tracing continue.
+            if (!headerContext.ShouldTrace)
+            {
+                TraceOptions options = rateLimitingFactory.CreateOptions();


...oud.Diagnostics.AspNetCore/Google.Cloud.Diagnostics.AspNetCore/Trace/CloudTraceMiddleware.cs

+    /// and trace the time taken for the next delegate to run.  The time taken and metadata
+    /// will be sent to the Stackdriver Trace API.
+    /// </summary>
+    public sealed class CloudTraceMiddleware


apis/Google.Cloud.Diagnostics.AspNetCore/Google.Cloud.Diagnostics.AspNetCore/Trace/Labels.cs

+        /// <summary>
+        /// Creates a map of labels to represent a <see cref="StackTrace"/> for a span.
+        /// </summary>
+        public static Dictionary<string, string> FromStackTrace(StackTrace stackTrace) =>


iantalarico

Thank Jon, PTAL

...ostics.AspNetCore/Google.Cloud.Diagnostics.AspNetCore.Tests/Trace/CloudTraceExtensionTest.cs

+{
+    public class CloudTraceExtensionTest
+    {
+        private const string projectId = "pid";


...ostics.AspNetCore/Google.Cloud.Diagnostics.AspNetCore.Tests/Trace/CloudTraceExtensionTest.cs

+            TraceHeaderContext context, bool rateLimiterShouldTrace)
+        {
+            Mock<IConsumer<TraceProto>> mockConsumer = new Mock<IConsumer<TraceProto>>();
+            Mock<IServiceProvider> mockProvider = new Mock<IServiceProvider>();


...loud.Diagnostics.AspNetCore/Google.Cloud.Diagnostics.AspNetCore/Trace/CloudTraceExtension.cs

+            var traceIdFactory = provider.GetService<TraceIdFactory>();
+            var consumer = provider.GetService<IConsumer<TraceProto>>();
+
+            // Check that we have all the needed services.


...loud.Diagnostics.AspNetCore/Google.Cloud.Diagnostics.AspNetCore/Trace/CloudTraceExtension.cs

+                throw new InvalidOperationException("Ensure Google Cloud Trace is properly set up.");
+            }
+
+            // If the trace header says to trace or if the rate limiter allows tracing continue.


...loud.Diagnostics.AspNetCore/Google.Cloud.Diagnostics.AspNetCore/Trace/CloudTraceExtension.cs

+            // If the trace header says to trace or if the rate limiter allows tracing continue.
+            if (!headerContext.ShouldTrace)
+            {
+                TraceOptions options = rateLimitingFactory.CreateOptions();


...oud.Diagnostics.AspNetCore/Google.Cloud.Diagnostics.AspNetCore/Trace/CloudTraceMiddleware.cs

+    /// and trace the time taken for the next delegate to run.  The time taken and metadata
+    /// will be sent to the Stackdriver Trace API.
+    /// </summary>
+    public sealed class CloudTraceMiddleware


apis/Google.Cloud.Diagnostics.AspNetCore/Google.Cloud.Diagnostics.AspNetCore/Trace/Labels.cs

+        /// <summary>
+        /// Creates a map of labels to represent a <see cref="StackTrace"/> for a span.
+        /// </summary>
+        public static Dictionary<string, string> FromStackTrace(StackTrace stackTrace) =>


...ostics.AspNetCore/Google.Cloud.Diagnostics.AspNetCore.Tests/Trace/CloudTraceExtensionTest.cs

+            TraceHeaderContext context, bool rateLimiterShouldTrace)
+        {
+            Mock<IConsumer<TraceProto>> mockConsumer = new Mock<IConsumer<TraceProto>>();
+            Mock<IServiceProvider> mockProvider = new Mock<IServiceProvider>();


...oud.Diagnostics.AspNetCore/Google.Cloud.Diagnostics.AspNetCore/Trace/CloudTraceMiddleware.cs

+    /// and trace the time taken for the next delegate to run.  The time taken and metadata
+    /// will be sent to the Stackdriver Trace API.
+    /// </summary>
+    public sealed class CloudTraceMiddleware


...loud.Diagnostics.AspNetCore/Google.Cloud.Diagnostics.AspNetCore/Trace/CloudTraceExtension.cs

+            GaxPreconditions.CheckState(consumer != null, 
+                string.Format(message, typeof(TraceProto).GetType()));
+
+            // If the trace header and rate limiter say not to trace, return a no-op tracer.


iantalarico

Thanks Jon, PTAL

...ostics.AspNetCore/Google.Cloud.Diagnostics.AspNetCore.Tests/Trace/CloudTraceExtensionTest.cs

+            TraceHeaderContext context, bool rateLimiterShouldTrace)
+        {
+            Mock<IConsumer<TraceProto>> mockConsumer = new Mock<IConsumer<TraceProto>>();
+            Mock<IServiceProvider> mockProvider = new Mock<IServiceProvider>();


...loud.Diagnostics.AspNetCore/Google.Cloud.Diagnostics.AspNetCore/Trace/CloudTraceExtension.cs

+            GaxPreconditions.CheckState(consumer != null, 
+                string.Format(message, typeof(TraceProto).GetType()));
+
+            // If the trace header and rate limiter say not to trace, return a no-op tracer.


...oud.Diagnostics.AspNetCore/Google.Cloud.Diagnostics.AspNetCore/Trace/CloudTraceMiddleware.cs

+    /// and trace the time taken for the next delegate to run.  The time taken and metadata
+    /// will be sent to the Stackdriver Trace API.
+    /// </summary>
+    public sealed class CloudTraceMiddleware


jskeet

See comments, but feel free to merge.

...ostics.AspNetCore/Google.Cloud.Diagnostics.AspNetCore.Tests/Trace/CloudTraceExtensionTest.cs

+            TraceHeaderContext context, bool rateLimiterShouldTrace)
+        {
+            Mock<IConsumer<TraceProto>> mockConsumer = new Mock<IConsumer<TraceProto>>();
+            Mock<IServiceProvider> mockProvider = new Mock<IServiceProvider>();


...ostics.AspNetCore/Google.Cloud.Diagnostics.AspNetCore.Tests/Trace/CloudTraceExtensionTest.cs

-        private const string ProjectId = "pid";
-        private const string TraceId = "105445aa7843bc8bf206b12000100f00";
-        private const ulong SpanId = 81237123;
+        private const string projectId = "pid";


...loud.Diagnostics.AspNetCore/Google.Cloud.Diagnostics.AspNetCore/Trace/CloudTraceExtension.cs

+            GaxPreconditions.CheckState(consumer != null, 
+                string.Format(message, typeof(TraceProto).GetType()));
+
+            // If the trace header and rate limiter say not to trace, return a no-op tracer.


...oud.Diagnostics.AspNetCore/Google.Cloud.Diagnostics.AspNetCore/Trace/CloudTraceMiddleware.cs

+    /// and trace the time taken for the next delegate to run.  The time taken and metadata
+    /// will be sent to the Stackdriver Trace API.
+    /// </summary>
+    internal sealed class CloudTraceMiddleware


# Conflicts: # apis/Google.Cloud.Diagnostics.AspNetCore/Google.Cloud.Diagnostics.AspNetCore.Snippets/AspNetCoreSnippets.cs # apis/Google.Cloud.Diagnostics.AspNetCore/docs/index.md

iantalarico

Thanks Jon, sorry about the odd rename change...

Source-Link: googleapis/googleapis@249e9a1 Source-Link: googleapis/googleapis-gen@1f2c41b Copy-Tag: eyJwIjoiYXBpcy9Hb29nbGUuQ2xvdWQuQ29tcHV0ZS5WMS8uT3dsQm90LnlhbWwiLCJoIjoiMWYyYzQxYmJmZTA5NzYxNmNlYjFlOWZiNjc4NzJhNmJiMWRjNTlkNCJ9

Changes in this release: ### New features - Update compute API to revision 20220112 ([issue 700](googleapis#700)) ([commit ebe518a](googleapis@ebe518a)) - The original commit contained breaking changes, but they were reverted in [commit 50ea200](googleapis@50ea200)

Changes in this release: ### New features - Update compute API to revision 20220112 ([issue 700](#700)) ([commit ebe518a](ebe518a)) - The original commit contained breaking changes, but they were reverted in [commit 50ea200](50ea200)

iantalarico added 14 commits January 13, 2017 14:01

Partial factor out of common trace code.

204c36c

Merge branch 'master' into tracing-core

5399958

# Conflicts: # apis/Google.Cloud.Diagnostics.AspNet/Google.Cloud.Diagnostics.AspNet/Trace/Labels.cs

Partial asp.net core tracing

7b8e205

Small naming changes

fbdf077

Add comments to the asp.net core trace files

ac5c69a

Add asp.net core trace snippets

14712eb

Add asp.net core trace docs

348327d

Use HttpContextAccessor to get HttpContext

f2d7a74

Unit tests for asp.net core trace

7b1c6a7

Add http context accessor as it may not always be added

b4fcce2

Fix bad merge

43926e1

Fix bad merge

965d136

Fix bad merge

27c0685

iantalarico requested review from jskeet and evildour January 20, 2017 16:58

evildour reviewed Jan 20, 2017

View reviewed changes

Changes from PR# 700

e7e3731

iantalarico commented Jan 20, 2017

View reviewed changes

evildour reviewed Jan 20, 2017

View reviewed changes

Changes from PR googleapis#700

33bbb85

evildour reviewed Jan 20, 2017

View reviewed changes

Fix broken test.

05c5001

jskeet reviewed Jan 23, 2017

View reviewed changes

Changes from PR googleapis#700

bb15abe

iantalarico commented Jan 23, 2017

View reviewed changes

jskeet reviewed Jan 23, 2017

View reviewed changes

Changes from PR googleapis#700

fcfa4a5

iantalarico commented Jan 23, 2017

View reviewed changes

jskeet approved these changes Jan 23, 2017

View reviewed changes

iantalarico added 2 commits January 23, 2017 12:36

Changes from PR googleapis#700

f996b40

Merge branch 'master' into trace-core-2

8d0fb69

# Conflicts: # apis/Google.Cloud.Diagnostics.AspNetCore/Google.Cloud.Diagnostics.AspNetCore.Snippets/AspNetCoreSnippets.cs # apis/Google.Cloud.Diagnostics.AspNetCore/docs/index.md

iantalarico commented Jan 23, 2017

View reviewed changes

iantalarico merged commit 94bf514 into googleapis:master Jan 23, 2017

iantalarico deleted the trace-core-2 branch January 26, 2017 22:20

jskeet mentioned this pull request Feb 14, 2022

Release Google.Cloud.Compute.V1 version 1.1.0 #7969

Merged

Cloud Error Reporting Trace for ASP.NET Core #700

Cloud Error Reporting Trace for ASP.NET Core #700

Conversation

iantalarico commented Jan 20, 2017

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

jskeet commented Jan 20, 2017

iantalarico left a comment

Choose a reason for hiding this comment

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

evildour left a comment

Choose a reason for hiding this comment

This comment was marked as spam.

This comment was marked as spam.

iantalarico commented Jan 20, 2017

evildour left a comment

Choose a reason for hiding this comment

jskeet left a comment

Choose a reason for hiding this comment

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

iantalarico left a comment

Choose a reason for hiding this comment

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

iantalarico left a comment

Choose a reason for hiding this comment

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

jskeet left a comment

Choose a reason for hiding this comment

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

iantalarico left a comment

Choose a reason for hiding this comment