Enable automatic project id detection from monitored resources for GAE and GCE. #826

iantalarico · 2017-03-01T21:41:04Z

This also refactors Error Reporting Options and Event Targets to make them more consistent and simple to use.

…E and GCE. This also refactors Error Reporting Options and Event Targets to make them more consistent and simple to use.

jskeet

Only skimmed, but this seems reasonable... how easy is it to detect non-dev environment in ASP.NET? I think I know where to do it all in Core, and we might want to put that in the snippets too - we probably don't want tracing when running locally, and we won't be able to detect the project id.

...Google.Cloud.Diagnostics.AspNetCore/ErrorReporting/ErrorReportingExceptionLoggerExtension.cs

+        /// <summary>
+        /// Uses middleware that will report all uncaught exceptions to the Stackdriver
+        /// Error Reporting API.
+        /// <para />


apis/Google.Cloud.Diagnostics.Common/Google.Cloud.Diagnostics.Common/CommonUtils.cs

+                {
+                    Debug.WriteLine("Google Cloud Platfrom project ID mismatch. " +
+                        $"Project Id parameter '{projectId}' does not match " +
+                        $"resource project ID '{resourceProjectId}' " +


iantalarico · 2017-03-01T22:26:50Z

I created #829 to track your comment. I think this would make development much smoother with support like this.

...stics.AspNet/Google.Cloud.Diagnostics.AspNet/ErrorReporting/ErrorReportingExceptionFilter.cs

@@ -70,15 +70,32 @@ public class ErrorReportingExceptionFilter : IExceptionFilter, IDisposable
        public static ErrorReportingExceptionFilter Create(string projectId, string serviceName, string version,
            ErrorReportingOptions options = null)
        {
-            GaxPreconditions.CheckNotNullOrEmpty(projectId, nameof(projectId));


...stics.AspNet/Google.Cloud.Diagnostics.AspNet/ErrorReporting/ErrorReportingExceptionLogger.cs

@@ -67,15 +67,30 @@ public sealed class ErrorReportingExceptionLogger : ExceptionLogger, IDisposable
        public static ErrorReportingExceptionLogger Create(string projectId, string serviceName, string version,
            ErrorReportingOptions options = null)
        {
-            GaxPreconditions.CheckNotNullOrEmpty(projectId, nameof(projectId));


...stics.AspNet/Google.Cloud.Diagnostics.AspNet/ErrorReporting/ErrorReportingExceptionFilter.cs

+        /// defined by <see cref="GoogleCredential.GetApplicationDefaultAsync"/>.
+        /// <para>
+        /// Can be used when running on Google App Engine or Google Compute Engine.
+        /// The Google Cloud Platform project to report error to will detected from the


...stics.AspNet/Google.Cloud.Diagnostics.AspNet/ErrorReporting/ErrorReportingExceptionLogger.cs

+        /// defined by <see cref="GoogleCredential.GetApplicationDefaultAsync"/>.
+        /// <para>
+        /// Can be used when running on Google App Engine or Google Compute Engine.
+        /// The Google Cloud Platform project to report error to will detected from the


apis/Google.Cloud.Diagnostics.AspNet/Google.Cloud.Diagnostics.AspNet/Trace/CloudTrace.cs

        /// <param name="application">The Http application.</param>
+        /// <param name="projectId">Optional if running on Google App Engine or Google Compute Engine.
+        ///     The Google Cloud Platform project ID. If running on GAE or GCE the project ID will be


...Google.Cloud.Diagnostics.AspNetCore/ErrorReporting/ErrorReportingExceptionLoggerExtension.cs

@@ -59,14 +59,34 @@ public static class ErrorReportingExceptionLoggerExtension
            this IApplicationBuilder app, string projectId, string serviceName, string version,
            ErrorReportingOptions options = null)
        {
-            GaxPreconditions.CheckNotNullOrEmpty(projectId, nameof(projectId));


...Google.Cloud.Diagnostics.AspNetCore/ErrorReporting/ErrorReportingExceptionLoggerExtension.cs

+        /// Error Reporting API.
+        /// <para>
+        /// Can be used when running on Google App Engine or Google Compute Engine.
+        /// The Google Cloud Platform project to report error to will detected from the


apis/Google.Cloud.Diagnostics.AspNet/Google.Cloud.Diagnostics.AspNet/Trace/CloudTrace.cs

+            // Trace does not use monitored resources, only detect the monitored resource if
+            // a project id is needed.
+            projectId = projectId ?? CommonUtils.GetAndCheckProjectId(
+                projectId, MonitoredResourceBuilder.FromPlatform());


apis/Google.Cloud.Diagnostics.Common/Google.Cloud.Diagnostics.Common/CommonUtils.cs

+        /// If the string project id is null and the MonitoredResource does not contain a project id
+        /// then an <see cref="InvalidOperationException"/> is thrown.
+        /// </term>
+        /// </list>


iantalarico

Thanks Mike, PTAL

...stics.AspNet/Google.Cloud.Diagnostics.AspNet/ErrorReporting/ErrorReportingExceptionFilter.cs

@@ -70,15 +70,32 @@ public class ErrorReportingExceptionFilter : IExceptionFilter, IDisposable
        public static ErrorReportingExceptionFilter Create(string projectId, string serviceName, string version,
            ErrorReportingOptions options = null)
        {
-            GaxPreconditions.CheckNotNullOrEmpty(projectId, nameof(projectId));


...stics.AspNet/Google.Cloud.Diagnostics.AspNet/ErrorReporting/ErrorReportingExceptionFilter.cs

+        /// defined by <see cref="GoogleCredential.GetApplicationDefaultAsync"/>.
+        /// <para>
+        /// Can be used when running on Google App Engine or Google Compute Engine.
+        /// The Google Cloud Platform project to report error to will detected from the


...stics.AspNet/Google.Cloud.Diagnostics.AspNet/ErrorReporting/ErrorReportingExceptionLogger.cs

@@ -67,15 +67,30 @@ public sealed class ErrorReportingExceptionLogger : ExceptionLogger, IDisposable
        public static ErrorReportingExceptionLogger Create(string projectId, string serviceName, string version,
            ErrorReportingOptions options = null)
        {
-            GaxPreconditions.CheckNotNullOrEmpty(projectId, nameof(projectId));


...stics.AspNet/Google.Cloud.Diagnostics.AspNet/ErrorReporting/ErrorReportingExceptionLogger.cs

+        /// defined by <see cref="GoogleCredential.GetApplicationDefaultAsync"/>.
+        /// <para>
+        /// Can be used when running on Google App Engine or Google Compute Engine.
+        /// The Google Cloud Platform project to report error to will detected from the


apis/Google.Cloud.Diagnostics.AspNet/Google.Cloud.Diagnostics.AspNet/Trace/CloudTrace.cs

        /// <param name="application">The Http application.</param>
+        /// <param name="projectId">Optional if running on Google App Engine or Google Compute Engine.
+        ///     The Google Cloud Platform project ID. If running on GAE or GCE the project ID will be


...Google.Cloud.Diagnostics.AspNetCore/ErrorReporting/ErrorReportingExceptionLoggerExtension.cs

@@ -59,14 +59,34 @@ public static class ErrorReportingExceptionLoggerExtension
            this IApplicationBuilder app, string projectId, string serviceName, string version,
            ErrorReportingOptions options = null)
        {
-            GaxPreconditions.CheckNotNullOrEmpty(projectId, nameof(projectId));


...Google.Cloud.Diagnostics.AspNetCore/ErrorReporting/ErrorReportingExceptionLoggerExtension.cs

+        /// Error Reporting API.
+        /// <para>
+        /// Can be used when running on Google App Engine or Google Compute Engine.
+        /// The Google Cloud Platform project to report error to will detected from the


apis/Google.Cloud.Diagnostics.AspNet/Google.Cloud.Diagnostics.AspNet/Trace/CloudTrace.cs

+            // Trace does not use monitored resources, only detect the monitored resource if
+            // a project id is needed.
+            projectId = projectId ?? CommonUtils.GetAndCheckProjectId(
+                projectId, MonitoredResourceBuilder.FromPlatform());


apis/Google.Cloud.Diagnostics.Common/Google.Cloud.Diagnostics.Common/CommonUtils.cs

+        /// If the string project id is null and the MonitoredResource does not contain a project id
+        /// then an <see cref="InvalidOperationException"/> is thrown.
+        /// </term>
+        /// </list>


apis/Google.Cloud.Diagnostics.AspNet/Google.Cloud.Diagnostics.AspNet/Trace/CloudTrace.cs

@@ -121,7 +121,7 @@ private CloudTrace(string projectId, TraceConfiguration config = null, TraceServ
        /// Initialize tracing for this application.
        /// </summary>
        /// <param name="application">The Http application.</param>
-        /// <param name="projectId">Optional if running on Google App Engine or Google Compute Engine.
+        /// <param name="projectId">Optional if unspecified and running on Google App Engine or Google Compute Engine.


apis/Google.Cloud.Diagnostics.Common/Google.Cloud.Diagnostics.Common/CommonUtils.cs

        {
+            monitoredResource = monitoredResource ?? MonitoredResourceBuilder.FromPlatform();


iantalarico

Fixed, thanks!

apis/Google.Cloud.Diagnostics.Common/Google.Cloud.Diagnostics.Common/CommonUtils.cs

        {
+            monitoredResource = monitoredResource ?? MonitoredResourceBuilder.FromPlatform();


apis/Google.Cloud.Diagnostics.AspNet/Google.Cloud.Diagnostics.AspNet/Trace/CloudTrace.cs

@@ -121,7 +121,7 @@ private CloudTrace(string projectId, TraceConfiguration config = null, TraceServ
        /// Initialize tracing for this application.
        /// </summary>
        /// <param name="application">The Http application.</param>
-        /// <param name="projectId">Optional if running on Google App Engine or Google Compute Engine.
+        /// <param name="projectId">Optional if unspecified and running on Google App Engine or Google Compute Engine.


evildour

Just one additional comment. Otherwise, LGTM.

...oogle.Cloud.Diagnostics.Common/Google.Cloud.Diagnostics.Common/ErrorReporting/EventTarget.cs

        {
            return new EventTarget
            {
+                // The Error Reporting API does not use monitored resources, only detect the monitored
+                // resource if a project id is needed.
+                ProjectId = projectId ?? CommonUtils.GetAndCheckProjectId(projectId),


apis/Google.Cloud.Diagnostics.AspNet/Google.Cloud.Diagnostics.AspNet/Trace/CloudTrace.cs

@@ -132,8 +132,7 @@ public static void Initialize(HttpApplication application, string projectId = nu

            // Trace does not use monitored resources, only detect the monitored resource if
            // a project id is needed.
-            projectId = projectId ?? CommonUtils.GetAndCheckProjectId(
-                projectId, MonitoredResourceBuilder.FromPlatform());
+            projectId = projectId ?? CommonUtils.GetAndCheckProjectId(projectId);


# Conflicts: # apis/Google.Cloud.Diagnostics.AspNetCore/Google.Cloud.Diagnostics.AspNetCore/Trace/CloudTraceExtension.cs

iantalarico added 4 commits March 1, 2017 13:21

Enable automatic project id detection from monitored resources for GA…

4ad1eac

…E and GCE. This also refactors Error Reporting Options and Event Targets to make them more consistent and simple to use.

Fix ASP.NET build

07b1af1

Update CommonUtil comments

9bb66a5

Add comment to event target

38468ed

iantalarico requested review from jskeet, chrisdunelm and evildour March 1, 2017 21:41

jskeet approved these changes Mar 1, 2017

View reviewed changes

Changes from PR googleapis#826

eca13a3

evildour reviewed Mar 5, 2017

View reviewed changes

Changes from PR googleapis#826

17cade1

iantalarico commented Mar 6, 2017

View reviewed changes

evildour reviewed Mar 7, 2017

View reviewed changes

Changes from PR googleapis#826

8921d54

iantalarico commented Mar 7, 2017

View reviewed changes

Fix type

0bfa751

evildour approved these changes Mar 7, 2017

View reviewed changes

SurferJeffAtGoogle reviewed Mar 7, 2017

View reviewed changes

iantalarico added 2 commits March 7, 2017 10:13

Changes from PR # 826

0ed89b4

Merge branch 'master' into auto-project-id

edfe225

# Conflicts: # apis/Google.Cloud.Diagnostics.AspNetCore/Google.Cloud.Diagnostics.AspNetCore/Trace/CloudTraceExtension.cs

iantalarico merged commit 97ddf80 into googleapis:master Mar 9, 2017

iantalarico deleted the auto-project-id branch March 9, 2017 20:00

		{
		monitoredResource = monitoredResource ?? MonitoredResourceBuilder.FromPlatform();

Enable automatic project id detection from monitored resources for GAE and GCE. #826

Enable automatic project id detection from monitored resources for GAE and GCE. #826

Conversation

iantalarico commented Mar 1, 2017

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.

iantalarico commented Mar 1, 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.

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.

evildour 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.