feature: remove labels and traceId - put them in payload (#224)

Added new constants
Improved (fixed) HTTP logging logic
More doc cleanup

Author: kevmoo

pkgs/google_cloud/CHANGELOG.md

@@ -1,20 +1,29 @@
-## 0.4.0-wip
+## 0.4.0
 
-### BREAKING CHANGES
+### `constants.dart`
 
-- Renamed `RequestLogger` to `CloudLogger` and moved it to
-  `package:google_cloud/general.dart`.
+- Added `logSpanIdKey`, `logTraceKey`, and `logTraceSampledKey`.
 
-### New Features
+### `general.dart`
 
-- `CloudLogger` is no longer abstract and has a default implementation that
-  prints to stdout.
-- Added `payload`, `labels`, and `stackTrace` named parameters to
-  `CloudLogger` functions as well as `structuredLogEntry`.
+- **BREAKING** `structuredLogEntry` removed `traceId` parameter. Use the *new*
+  `payload` parameter with the `logTraceKey` constant as a key.
 - Hardened structured log JSON serialization with automatic fallback mechanisms
   to safely handle native `toJson()` implementations and circular references
   without failing.
 
+- Added `CloudLogger` (renamed from `RequestLogger`).
+  - Added optional `payload` and `stackTrace` named parameters to
+    `CloudLogger` functions.
+  - `CloudLogger` is no longer abstract and has a default implementation that
+    prints to stdout.
+
+### `http_serving.dart`
+
+- **BREAKING** Renamed `RequestLogger` to `CloudLogger` and moved it to
+  `package:google_cloud/general.dart`.
+- Refactored HTTP logging logic to handle `spanId` and `traceSampled`.
+
 ## 0.3.1
 
 - Fix a bug where `projectIdFromGcloudConfig()` used the incorrect gcloud shell

pkgs/google_cloud/lib/constants.dart

@@ -15,15 +15,4 @@
 /// Constants for Google Cloud Platform integration.
 library;
 
-export 'src/constants.dart'
-    show
-        cloudTraceContextHeader,
-        configurationEnvironmentVariable,
-        credentialsPathEnvironmentVariable,
-        defaultListenPort,
-        metadataFlavorHeaders,
-        portEnvironmentVariable,
-        projectIdEnvironmentVariable,
-        projectIdEnvironmentVariableOptions,
-        revisionEnvironmentVariable,
-        serviceEnvironmentVariable;
+export 'src/constants.dart';

pkgs/google_cloud/lib/http_serving.dart

@@ -61,3 +61,4 @@ export 'src/serving/http_logging.dart'
         currentLogger;
 export 'src/serving/serve.dart' show listenPortFromEnvironment, serveHandler;
 export 'src/serving/terminate.dart' show waitForTerminate;
+export 'src/serving/trace_context_data.dart' show TraceContextData;

pkgs/google_cloud/lib/src/constants.dart

@@ -55,10 +55,32 @@ const cloudTraceContextHeader = 'x-cloud-trace-context';
 const metadataFlavorHeaders = {'Metadata-Flavor': 'Google'};
 
 /// The name of the Cloud Run service being run.
+///
+/// See https://docs.cloud.google.com/run/docs/container-contract#services-env-vars
 const serviceEnvironmentVariable = 'K_SERVICE';
 
 /// The name of the Cloud Run revision being run.
+///
+/// See https://docs.cloud.google.com/run/docs/container-contract#services-env-vars
 const revisionEnvironmentVariable = 'K_REVISION';
 
 /// The name of the Cloud Run configuration being run.
+///
+/// See https://docs.cloud.google.com/run/docs/container-contract#services-env-vars
 const configurationEnvironmentVariable = 'K_CONFIGURATION';
+
+/// The `payload` key used to correlate log entries with Cloud Trace.
+///
+/// See https://docs.cloud.google.com/logging/docs/agent/logging/configuration#special-fields
+const logTraceKey = 'logging.googleapis.com/trace';
+
+/// The `payload` key used to correlate log entries with a specific span within
+/// a Cloud Trace.
+///
+/// See https://docs.cloud.google.com/logging/docs/agent/logging/configuration#special-fields
+const logSpanIdKey = 'logging.googleapis.com/spanId';
+
+/// The `payload` key used to indicate whether a trace is sampled.
+///
+/// See https://docs.cloud.google.com/logging/docs/agent/logging/configuration#special-fields
+const logTraceSampledKey = 'logging.googleapis.com/trace_sampled';

pkgs/google_cloud/lib/src/logger.dart

@@ -66,7 +66,7 @@ abstract base class CloudLogger {
   /// This logger prints messages to the console.
   ///
   /// The output format is:
-  /// `[SEVERITY_NAME: ]<message>[ payload][ labels][\nstack_trace]`
+  /// `[SEVERITY_NAME: ]<message>[ payload][\nstack_trace]`
   ///
   /// The `SEVERITY_NAME: ` prefix is omitted when the severity is
   /// [LogSeverity.defaultSeverity].
@@ -85,7 +85,6 @@ abstract base class CloudLogger {
     Object message,
     LogSeverity severity, {
     Map<String, Object?>? payload,
-    Map<String, String>? labels,
     StackTrace? stackTrace,
   });
 
@@ -95,45 +94,31 @@ abstract base class CloudLogger {
   void debug(
     Object message, {
     Map<String, Object?>? payload,
-    Map<String, String>? labels,
     StackTrace? stackTrace,
-  }) => log(
-    message,
-    LogSeverity.debug,
-    payload: payload,
-    labels: labels,
-    stackTrace: stackTrace,
-  );
+  }) =>
+      log(message, LogSeverity.debug, payload: payload, stackTrace: stackTrace);
 
   /// Logs [message] at [LogSeverity.info] severity.
   ///
   /// {@macro google_cloud.CloudLogger.log_params}
   void info(
     Object message, {
     Map<String, Object?>? payload,
-    Map<String, String>? labels,
     StackTrace? stackTrace,
-  }) => log(
-    message,
-    LogSeverity.info,
-    payload: payload,
-    labels: labels,
-    stackTrace: stackTrace,
-  );
+  }) =>
+      log(message, LogSeverity.info, payload: payload, stackTrace: stackTrace);
 
   /// Logs [message] at [LogSeverity.notice] severity.
   ///
   /// {@macro google_cloud.CloudLogger.log_params}
   void notice(
     Object message, {
     Map<String, Object?>? payload,
-    Map<String, String>? labels,
     StackTrace? stackTrace,
   }) => log(
     message,
     LogSeverity.notice,
     payload: payload,
-    labels: labels,
     stackTrace: stackTrace,
   );
 
@@ -143,13 +128,11 @@ abstract base class CloudLogger {
   void warning(
     Object message, {
     Map<String, Object?>? payload,
-    Map<String, String>? labels,
     StackTrace? stackTrace,
   }) => log(
     message,
     LogSeverity.warning,
     payload: payload,
-    labels: labels,
     stackTrace: stackTrace,
   );
 
@@ -159,29 +142,21 @@ abstract base class CloudLogger {
   void error(
     Object message, {
     Map<String, Object?>? payload,
-    Map<String, String>? labels,
     StackTrace? stackTrace,
-  }) => log(
-    message,
-    LogSeverity.error,
-    payload: payload,
-    labels: labels,
-    stackTrace: stackTrace,
-  );
+  }) =>
+      log(message, LogSeverity.error, payload: payload, stackTrace: stackTrace);
 
   /// Logs [message] at [LogSeverity.critical] severity.
   ///
   /// {@macro google_cloud.CloudLogger.log_params}
   void critical(
     Object message, {
     Map<String, Object?>? payload,
-    Map<String, String>? labels,
     StackTrace? stackTrace,
   }) => log(
     message,
     LogSeverity.critical,
     payload: payload,
-    labels: labels,
     stackTrace: stackTrace,
   );
 
@@ -191,29 +166,21 @@ abstract base class CloudLogger {
   void alert(
     Object message, {
     Map<String, Object?>? payload,
-    Map<String, String>? labels,
     StackTrace? stackTrace,
-  }) => log(
-    message,
-    LogSeverity.alert,
-    payload: payload,
-    labels: labels,
-    stackTrace: stackTrace,
-  );
+  }) =>
+      log(message, LogSeverity.alert, payload: payload, stackTrace: stackTrace);
 
   /// Logs [message] at [LogSeverity.emergency] severity.
   ///
   /// {@macro google_cloud.CloudLogger.log_params}
   void emergency(
     Object message, {
     Map<String, Object?>? payload,
-    Map<String, String>? labels,
     StackTrace? stackTrace,
   }) => log(
     message,
     LogSeverity.emergency,
     payload: payload,
-    labels: labels,
     stackTrace: stackTrace,
   );
 }
@@ -228,18 +195,16 @@ final class _DefaultLogger extends CloudLogger {
     Object message,
     LogSeverity severity, {
     Map<String, Object?>? payload,
-    Map<String, String>? labels,
     StackTrace? stackTrace,
   }) {
     final payloadStr = payload != null && payload.isNotEmpty ? ' $payload' : '';
-    final labelsStr = labels != null && labels.isNotEmpty ? ' $labels' : '';
     final traceStr = stackTrace != null
         ? '\n${formatStackTrace(stackTrace)}'
         : '';
     if (severity == LogSeverity.defaultSeverity) {
-      print('$message$payloadStr$labelsStr$traceStr');
+      print('$message$payloadStr$traceStr');
     } else {
-      print('${severity.name}: $message$payloadStr$labelsStr$traceStr');
+      print('${severity.name}: $message$payloadStr$traceStr');
     }
   }
 }
@@ -249,6 +214,5 @@ bool frameFolder(Frame frame) =>
     frame.isCore || frame.package == 'google_cloud';
 
 @internal
-Chain formatStackTrace(StackTrace? stackTrace) =>
-    (stackTrace == null ? Chain.current() : Chain.forTrace(stackTrace))
-        .foldFrames(frameFolder, terse: true);
+Chain formatStackTrace(StackTrace stackTrace) =>
+    Chain.forTrace(stackTrace).foldFrames(frameFolder, terse: true);

pkgs/google_cloud/lib/src/serving/http_logging.dart

@@ -23,6 +23,7 @@ import '../constants.dart';
 import '../logger.dart';
 import '../structured_logging.dart';
 import 'bad_request_exception.dart';
+import 'trace_context_data.dart';
 
 export '../structured_logging.dart';
 
@@ -67,7 +68,7 @@ Handler _errorWriter(Handler innerHandler) => (request) async {
       error,
       if (error.innerError != null)
         '${error.innerError} (${error.innerError.runtimeType})',
-      formatStackTrace(error.innerStack ?? stack),
+      if (error.innerStack ?? stack case final s?) formatStackTrace(s),
     ];
 
     final bob = output
@@ -101,12 +102,13 @@ Middleware cloudLoggingMiddleware(String projectId) {
     // Add log correlation to nest all log messages beneath request log in
     // Log Viewer.
 
-    String? traceValue;
-
     final traceHeader = request.headers[cloudTraceContextHeader];
-    if (traceHeader != null) {
-      traceValue = 'projects/$projectId/traces/${traceHeader.split('/')[0]}';
-    }
+    final traceContext = traceHeader != null
+        ? TraceContextData.tryParse(
+            projectId: projectId,
+            traceHeader: traceHeader,
+          )
+        : null;
 
     String createErrorLogEntryFromRequest(
       Object error,
@@ -115,7 +117,7 @@ Middleware cloudLoggingMiddleware(String projectId) {
     ) => structuredLogEntry(
       '$error'.trim(),
       logSeverity,
-      traceId: traceValue,
+      payload: traceContext?.asPayloadMap(),
       stackTrace: stackTrace,
     );
 
@@ -126,7 +128,10 @@ Middleware cloudLoggingMiddleware(String projectId) {
     Zone.current
         .fork(
           zoneValues: {
-            _loggerKey: _CloudLogger(zone: currentZone, traceId: traceValue),
+            _loggerKey: _CloudLogger(
+              zone: currentZone,
+              traceContext: traceContext,
+            ),
           },
           specification: ZoneSpecification(
             handleUncaughtError: (self, parent, zone, error, stackTrace) {
@@ -163,7 +168,7 @@ Middleware cloudLoggingMiddleware(String projectId) {
               final logContent = structuredLogEntry(
                 line,
                 LogSeverity.info,
-                traceId: traceValue,
+                payload: traceContext?.asPayloadMap(),
               );
 
               // Serialize to a JSON string and output to parent zone.
@@ -203,12 +208,11 @@ final _loggerKey = Object();
 /// logging.
 final class _CloudLogger extends CloudLogger {
   final Zone zone;
-  final String? _traceId;
+
+  final TraceContextData? traceContext;
 
   /// Creates a new [_CloudLogger] that prints structured logs to [this.zone].
-  ///
-  /// If [_traceId] is provided, it is included in the log entry.
-  _CloudLogger({required this.zone, String? traceId}) : _traceId = traceId;
+  _CloudLogger({required this.zone, this.traceContext});
 
   /// If [message] is a [Map], it is used as the log entry payload. Otherwise,
   /// it is passed directly to [structuredLogEntry], which handles
@@ -218,15 +222,12 @@ final class _CloudLogger extends CloudLogger {
     Object message,
     LogSeverity severity, {
     Map<String, Object?>? payload,
-    Map<String, String>? labels,
     StackTrace? stackTrace,
   }) => zone.print(
     structuredLogEntry(
       message,
       severity,
-      payload: payload,
-      labels: labels,
-      traceId: _traceId,
+      payload: traceContext?.asPayloadMap(payload) ?? payload,
       stackTrace: stackTrace,
     ),
   );