[shivanee-p]

This pull request adds logNetworkAttempt to LoggerHelper for failed RPC attempts.

Key Changes:

• Added logNetworkAttempt in LoggerHelper.ts to emit a structured DEBUG log entry ('RPC attempt failed: %s') whenever an individual RPC transport attempt fails across promise, callback, stream, or synchronous execution boundaries.
• Populated response.payload on attempt debug logs using resolveResponsePayload(err?.metadata) when metadata is present, while omitting exception.type, exception.message, and exception.stacktrace.
• Extracted executeLoggedOperation and resolveLogSeverity in LoggerHelper.ts to share promise, callback, stream, and synchronous error interception between logCall and logNetworkAttempt.

[gemini-code-assist]

Code Review

This pull request refactors the logging utility by extracting common execution wrapping logic into executeLoggedOperation and introducing logNetworkAttempt to log individual RPC transport attempt failures at the DEBUG level. Feedback suggests adding a latch flag in logNetworkAttempt to prevent duplicate DEBUG logs if both callback and stream error events are triggered, and splitting the corresponding combined test into separate unit tests to verify each failure path independently.

[gemini-code-assist]

To prevent duplicate DEBUG logs for a single network attempt (for example, when both a callback and a stream error event are triggered), we should introduce a latch flag similar to hasLoggedL3 in logCall.

Suggested change

	// Snapshot attempt context in case async rejection loses active context.
	const initialContext = context.active();
	const emitL4DebugLog = (err: unknown) => {
	// Latch to emit at most one debug log per network attempt.
	let hasLoggedAttempt = false;
	// Snapshot attempt context in case async rejection loses active context.
	const initialContext = context.active();
	const emitL4DebugLog = (err: unknown) => {
	// Suppress duplicate emissions for the same attempt.
	if (hasLoggedAttempt) {
	return;
	}
	hasLoggedAttempt = true;

[gemini-code-assist]

With the introduction of the latch flag to prevent duplicate logging, we should split the combined callback and stream failure test into two separate, clean unit tests to verify each failure path independently.

    it('captures the attempt span context and logs on callback failures', () => {
      let receivedCallback: APICallback | undefined;

      traceCall(dynamicArgs, staticArgs, () =>
        traceAttempt(dynamicArgs, staticArgs, () =>
          logNetworkAttempt(
            dynamicArgs,
            staticArgs,
            loggedCallback => {
              receivedCallback = loggedCallback;
              return {ok: true};
            },
            false,
            () => {},
          ),
        ),
      );

      const attemptErr = new GoogleError('attempt failed');
      receivedCallback!(attemptErr);

      const spans = harness.getSpans('google-gax');
      assert.strictEqual(spans.length, 2);
      const attemptSpan = spans[0];

      assert.strictEqual(sink.logs.length, 1);
      const logEntry = sink.logs[0];
      assert.strictEqual(logEntry.fields.severity, 'DEBUG');
      assert.strictEqual(
        logEntry.fields.telemetryTraceId,
        attemptSpan.spanContext().traceId,
      );
      assert.strictEqual(
        logEntry.fields.telemetrySpanId,
        attemptSpan.spanContext().spanId,
      );
      assert.deepStrictEqual(logEntry.args, [
        'RPC attempt failed: %s',
        'attempt failed',
      ]);
    });

    it('captures the attempt span context and logs on stream failures', () => {
      const stream = new EventEmitter();

      traceCall(dynamicArgs, staticArgs, () =>
        traceAttempt(dynamicArgs, staticArgs, () =>
          logNetworkAttempt(
            dynamicArgs,
            staticArgs,
            () => stream,
            true,
          ),
        ),
      );

      const attemptErr = new GoogleError('attempt failed');
      stream.emit('error', attemptErr);

      const spans = harness.getSpans('google-gax');
      assert.strictEqual(spans.length, 2);
      const attemptSpan = spans[0];

      assert.strictEqual(sink.logs.length, 1);
      const logEntry = sink.logs[0];
      assert.strictEqual(logEntry.fields.severity, 'DEBUG');
      assert.strictEqual(
        logEntry.fields.telemetryTraceId,
        attemptSpan.spanContext().traceId,
      );
      assert.strictEqual(
        logEntry.fields.telemetrySpanId,
        attemptSpan.spanContext().spanId,
      );
      assert.deepStrictEqual(logEntry.args, [
        'RPC attempt failed: %s',
        'attempt failed',
      ]);
    });

[quirogas]

Reviewed #9757 against the Milestone 2 Error Logging design doc and live gRPC/REST (fallback: 'rest') error payloads. Approving to unblock merging once the 3 inline items below (and 1 minor nit) are addressed.

[quirogas]

Passing (err as {metadata?: unknown} | null | undefined)?.metadata directly to resolveResponsePayload for response.payload (L4 Attributes table: "The full raw response payload (if any).") fails in three real-world cases:

1. HTTP REST errors: On all HTTP GoogleErrors (fallbackRest.ts / fallbackServiceStub.ts), err.metadata is undefined, so response.payload is never populated on HTTP L4 logs.
2. Empty grpc.Metadata on pre-connection failures: @grpc/grpc-js attaches an empty new grpc.Metadata() to every ServiceError (even when no response arrived, such as ECONNREFUSED), which Metadata#toJSON() serializes as "response.payload": "{}" instead of omitting response.payload.
3. Populated grpc.Metadata array values: Unlike the plain object { 'x-goog-request-id': 'req-123' } mocked in test/unit/loggerHelper.ts, a real grpc.Metadata instance has a .toJSON() method that serializes header values as arrays ({"x-goog-request-id":["req-123"]}) unless .getMap() is unwrapped first.

Unwrap metadata.getMap(), ignore empty metadata maps, and fall back to err.statusDetails:

const rawMeta = (err as {metadata?: {getMap?: () => Record<string, unknown>}} | null)?.metadata;
const metaMap = typeof rawMeta?.getMap === 'function' ? rawMeta.getMap() : rawMeta;
const rawPayload = (metaMap && Object.keys(metaMap).length > 0 ? metaMap : undefined) ?? (err as {statusDetails?: unknown} | null)?.statusDetails;
const responsePayload = resolveResponsePayload(rawPayload);

[quirogas]

Unlike logCall (which has let hasLoggedL3 = false), logNetworkAttempt has no one-time error latch. In fallbackServiceStub.ts, when an HTTP streaming attempt fails, the stub invokes both callback(err) and streamArrayParser.emit('error', err) for the same attempt. Without a latch, a single failed attempt emits 2 duplicate DEBUG logs (which test/unit/loggerHelper.ts currently asserts as assert.strictEqual(sink.logs.length, 2)), whereas the Milestone 2 Design Doc (## Testing item #5) requires logging the error at most once per attempt. Move the let hasLogged = false latch inside executeLoggedOperation so both logCall and logNetworkAttempt emit at most 1 log per invocation.