Update comment and undo unnecessary logging (#13025)

This commit is contained in:
Tommaso Sciortino
2025-11-13 19:11:13 -08:00
committed by GitHub
parent ab11b2c27f
commit 2c5e09e1c3
4 changed files with 20 additions and 71 deletions

View File

@@ -575,6 +575,15 @@ export class GeminiChat {
content: responseText,
});
}
// Stream validation logic: A stream is considered successful if:
// 1. There's a tool call OR
// 2. A not MALFORMED_FUNCTION_CALL finish reason and a non-mepty resp
//
// We throw an error only when there's no tool call AND:
// - No finish reason, OR
// - MALFORMED_FUNCTION_CALL finish reason OR
// - Empty response text (e.g., only thoughts with no actual content)
if (!hasToolCall) {
if (!finishReason) {
throw new InvalidStreamError(

View File

@@ -442,7 +442,7 @@ export function logContentRetry(
attributes: event.toOpenTelemetryAttributes(config),
};
logger.emit(logRecord);
recordContentRetry(config, event.error_type);
recordContentRetry(config);
}
export function logContentRetryFailure(
@@ -458,7 +458,7 @@ export function logContentRetryFailure(
attributes: event.toOpenTelemetryAttributes(config),
};
logger.emit(logRecord);
recordContentRetryFailure(config, event.final_error_type);
recordContentRetryFailure(config);
}
export function logModelRouting(

View File

@@ -96,8 +96,6 @@ describe('Telemetry Metrics', () => {
let recordAgentRunMetricsModule: typeof import('./metrics.js').recordAgentRunMetrics;
let recordLinesChangedModule: typeof import('./metrics.js').recordLinesChanged;
let recordSlowRenderModule: typeof import('./metrics.js').recordSlowRender;
let recordContentRetryModule: typeof import('./metrics.js').recordContentRetry;
let recordContentRetryFailureModule: typeof import('./metrics.js').recordContentRetryFailure;
beforeEach(async () => {
vi.resetModules();
@@ -142,8 +140,6 @@ describe('Telemetry Metrics', () => {
recordAgentRunMetricsModule = metricsJsModule.recordAgentRunMetrics;
recordLinesChangedModule = metricsJsModule.recordLinesChanged;
recordSlowRenderModule = metricsJsModule.recordSlowRender;
recordContentRetryModule = metricsJsModule.recordContentRetry;
recordContentRetryFailureModule = metricsJsModule.recordContentRetryFailure;
const otelApiModule = await import('@opentelemetry/api');
@@ -1347,50 +1343,4 @@ describe('Telemetry Metrics', () => {
});
});
});
describe('recordContentRetry', () => {
it('does not record metrics if not initialized', () => {
const config = makeFakeConfig({});
recordContentRetryModule(config, 'NO_FINISH_REASON');
expect(mockCounterAddFn).not.toHaveBeenCalled();
});
it('records a content retry event with error type when initialized', () => {
const config = makeFakeConfig({});
initializeMetricsModule(config);
mockCounterAddFn.mockClear(); // Clear the session start call
recordContentRetryModule(config, 'MALFORMED_FUNCTION_CALL');
expect(mockCounterAddFn).toHaveBeenCalledWith(1, {
'session.id': 'test-session-id',
'installation.id': 'test-installation-id',
'user.email': 'test@example.com',
error_type: 'MALFORMED_FUNCTION_CALL',
});
});
});
describe('recordContentRetryFailure', () => {
it('does not record metrics if not initialized', () => {
const config = makeFakeConfig({});
recordContentRetryFailureModule(config, 'NO_RESPONSE_TEXT');
expect(mockCounterAddFn).not.toHaveBeenCalled();
});
it('records a content retry failure event with error type when initialized', () => {
const config = makeFakeConfig({});
initializeMetricsModule(config);
mockCounterAddFn.mockClear(); // Clear the session start call
recordContentRetryFailureModule(config, 'MALFORMED_FUNCTION_CALL');
expect(mockCounterAddFn).toHaveBeenCalledWith(1, {
'session.id': 'test-session-id',
'installation.id': 'test-installation-id',
'user.email': 'test@example.com',
error_type: 'MALFORMED_FUNCTION_CALL',
});
});
});
});

View File

@@ -136,17 +136,13 @@ const COUNTER_DEFINITIONS = {
description: 'Counts retries due to content errors (e.g., empty stream).',
valueType: ValueType.INT,
assign: (c: Counter) => (contentRetryCounter = c),
attributes: {} as {
error_type: string;
},
attributes: {} as Record<string, never>,
},
[CONTENT_RETRY_FAILURE_COUNT]: {
description: 'Counts occurrences of all content retries failing.',
valueType: ValueType.INT,
assign: (c: Counter) => (contentRetryFailureCounter = c),
attributes: {} as {
error_type: string;
},
attributes: {} as Record<string, never>,
},
[MODEL_ROUTING_FAILURE_COUNT]: {
description: 'Counts model routing failures.',
@@ -719,26 +715,20 @@ export function recordInvalidChunk(config: Config): void {
/**
* Records a metric for when a retry is triggered due to a content error.
*/
export function recordContentRetry(config: Config, errorType: string): void {
export function recordContentRetry(config: Config): void {
if (!contentRetryCounter || !isMetricsInitialized) return;
contentRetryCounter.add(1, {
...baseMetricDefinition.getCommonAttributes(config),
error_type: errorType,
});
contentRetryCounter.add(1, baseMetricDefinition.getCommonAttributes(config));
}
/**
* Records a metric for when all content error retries have failed for a request.
*/
export function recordContentRetryFailure(
config: Config,
errorType: string,
): void {
export function recordContentRetryFailure(config: Config): void {
if (!contentRetryFailureCounter || !isMetricsInitialized) return;
contentRetryFailureCounter.add(1, {
...baseMetricDefinition.getCommonAttributes(config),
error_type: errorType,
});
contentRetryFailureCounter.add(
1,
baseMetricDefinition.getCommonAttributes(config),
);
}
export function recordModelSlashCommand(