Skip to content
This repository was archived by the owner on Mar 11, 2026. It is now read-only.

Commit 6d470fc

Browse files
losalexgcf-owl-bot[bot]
losalex
and
gcf-owl-bot[bot]
authored
fix: Add support to set an instrumentation flag (#1279)
* fix: Add support to set an instrumentation flag * Fix a comment * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md * fix lint errors * Add a fix for partialSuccess to work properly. Refactor tests to adopt a change Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
1 parent 0bfe813 commit 6d470fc

5 files changed

Lines changed: 79 additions & 53 deletions

File tree

‎src/log-sync.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -413,7 +413,7 @@ class LogSync implements LogSeverityFunctions {
413413
this.formattedName_ = formatLogName(this.logging.projectId, this.name);
414414
try {
415415
// Make sure to add instrumentation info
416-
structuredEntries = populateInstrumentationInfo(entry).map(entry => {
416+
structuredEntries = populateInstrumentationInfo(entry)[0].map(entry => {
417417
if (!(entry instanceof Entry)) {
418418
entry = this.entry(entry);
419419
}

‎src/log.ts‎

Lines changed: 8 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,7 @@ import * as extend from 'extend';
2020
import {CallOptions} from 'google-gax';
2121
import {GetEntriesCallback, GetEntriesResponse, Logging} from '.';
2222
import {Entry, EntryJson, LogEntry} from './entry';
23-
import {
24-
populateInstrumentationInfo,
25-
getInstrumentationInfoStatus,
26-
} from './utils/instrumentation';
23+
import {populateInstrumentationInfo} from './utils/instrumentation';
2724
import {
2825
LogSeverityFunctions,
2926
assignSeverityToEntries,
@@ -962,23 +959,19 @@ class Log implements LogSeverityFunctions {
962959
entry: Entry | Entry[],
963960
opts?: WriteOptions | ApiResponseCallback
964961
): Promise<ApiResponse> {
965-
const isInfoAdded = getInstrumentationInfoStatus();
966962
const options = opts ? (opts as WriteOptions) : {};
967-
// If instrumentation info was not added, means that this is first time
968-
// log entry is written and that the instrumentation log entry could be
969-
// generated for this request. If yes, then make sure we set partialSuccess, so entire
970-
// request will make it through and only oversized entries will be dropped
971-
if (!isInfoAdded) {
972-
options.partialSuccess = true;
973-
}
974963
// Extract projectId & resource from Logging - inject & memoize if not.
975964
await this.logging.setProjectId();
976965
this.formattedName_ = formatLogName(this.logging.projectId, this.name);
977966
const resource = await this.getOrSetResource(options);
978967
// Extract & format additional context from individual entries. Make sure to add instrumentation info
979-
const decoratedEntries = this.decorateEntries(
980-
populateInstrumentationInfo(entry)
981-
);
968+
const info = populateInstrumentationInfo(entry);
969+
const decoratedEntries = this.decorateEntries(info[0]);
970+
// If instrumentation info was added make sure we set partialSuccess, so entire
971+
// request will make it through and only oversized entries will be dropped if any
972+
if (info[1]) {
973+
options.partialSuccess = true;
974+
}
982975
this.truncateEntries(decoratedEntries);
983976
// Clobber `labels` and `resource` fields with WriteOptions from the user.
984977
const reqOpts = extend(

‎src/utils/instrumentation.ts‎

Lines changed: 18 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -38,24 +38,20 @@ export const INSTRUMENTATION_SOURCE_KEY = 'instrumentation_source';
3838
export const NODEJS_LIBRARY_NAME_PREFIX = 'nodejs';
3939
export type InstrumentationInfo = {name: string; version: string};
4040

41-
/**
42-
* This method returns the status if instrumentation info was already added or not.
43-
* @returns true if the log record with instrumentation info was already added, false otherwise.
44-
*/
45-
export function getInstrumentationInfoStatus() {
46-
return global.instrumentationAdded;
47-
}
48-
4941
/**
5042
* This method helps to populate entries with instrumentation data
5143
* @param entry {Entry} The entry or array of entries to be populated with instrumentation info
52-
* @returns {Entry} Array of entries which contains an entry with current library instrumentation info
44+
* @returns [Entry[], boolean] Array of entries which contains an entry with current library
45+
* instrumentation info and boolean flag indicating if instrumentation was added or not in this call
5346
*/
54-
export function populateInstrumentationInfo(entry: Entry | Entry[]): Entry[] {
47+
export function populateInstrumentationInfo(
48+
entry: Entry | Entry[]
49+
): [Entry[], boolean] {
5550
// Update the flag indicating that instrumentation entry was already added once,
5651
// so any subsequent calls to this method will not add a separate instrumentation log entry
57-
let isWritten = global.instrumentationAdded;
58-
global.instrumentationAdded = true;
52+
let isWritten = setInstrumentationStatus(true);
53+
// Flag keeping track if this specific call added any instrumentation info
54+
let isInfoAdded = false;
5955
const entries: Entry[] = [];
6056
if (entry) {
6157
// eslint-disable-next-line @typescript-eslint/no-explicit-any
@@ -69,7 +65,7 @@ export function populateInstrumentationInfo(entry: Entry | Entry[]): Entry[] {
6965
validateAndUpdateInstrumentation(info);
7066
// Indicate that instrumentation info log entry already exists
7167
// and that current library info was added to existing log entry
72-
isWritten = true;
68+
isInfoAdded = isWritten = true;
7369
}
7470
entries.push(entryItem);
7571
}
@@ -79,8 +75,9 @@ export function populateInstrumentationInfo(entry: Entry | Entry[]): Entry[] {
7975
// instrumentation data for this library
8076
if (!isWritten) {
8177
entries.push(createDiagnosticEntry(undefined, undefined));
78+
isInfoAdded = true;
8279
}
83-
return entries;
80+
return [entries, isInfoAdded];
8481
}
8582

8683
/**
@@ -198,8 +195,12 @@ function isValidInfo(info: InstrumentationInfo) {
198195
}
199196

200197
/**
201-
* The helper method used to reset a status of a flag which indicates if instrumentation info already written or not.
198+
* The helper method used to set a status of a flag which indicates if instrumentation info already written or not.
199+
* @param value {boolean} The value to be set.
200+
* @returns The value of the flag before it is set.
202201
*/
203-
export function resetInstrumentationStatus() {
204-
global.instrumentationAdded = false;
202+
export function setInstrumentationStatus(value: boolean) {
203+
const status = global.instrumentationAdded;
204+
global.instrumentationAdded = value;
205+
return status;
205206
}

‎test/instrumentation.ts‎

Lines changed: 25 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ const LONG_VERSION_TEST = VERSION_TEST + '.0.0.0.0.0.0.0.0.11.1.1-ALPHA';
2929

3030
describe('instrumentation_info', () => {
3131
beforeEach(() => {
32-
instrumentation.resetInstrumentationStatus();
32+
instrumentation.setInstrumentationStatus(false);
3333
});
3434

3535
it('should generate library info properly by default', () => {
@@ -54,10 +54,11 @@ describe('instrumentation_info', () => {
5454
it('should add instrumentation log entry to the list', () => {
5555
const dummyEntry = createEntry(undefined, undefined);
5656
const entries = instrumentation.populateInstrumentationInfo(dummyEntry);
57-
assert.equal(entries.length, 2);
58-
assert.deepEqual(dummyEntry, entries[0]);
57+
assert.equal(entries[0].length, 2);
58+
assert.deepEqual(dummyEntry, entries[0][0]);
59+
assert.equal(true, entries[1]);
5960
assert.equal(
60-
entries[1].data?.[instrumentation.DIAGNOSTIC_INFO_KEY]?.[
61+
entries[0][1].data?.[instrumentation.DIAGNOSTIC_INFO_KEY]?.[
6162
instrumentation.INSTRUMENTATION_SOURCE_KEY
6263
]?.[0]?.[NAME],
6364
instrumentation.NODEJS_LIBRARY_NAME_PREFIX
@@ -67,27 +68,28 @@ describe('instrumentation_info', () => {
6768
it('should add instrumentation info to existing list', () => {
6869
const dummyEntry = createEntry(NODEJS_TEST, VERSION_TEST);
6970
const entries = instrumentation.populateInstrumentationInfo(dummyEntry);
70-
assert.equal(entries.length, 1);
71+
assert.equal(entries[0].length, 1);
72+
assert.equal(true, entries[1]);
7173
assert.equal(
72-
entries[0].data?.[instrumentation.DIAGNOSTIC_INFO_KEY]?.[
74+
entries[0][0].data?.[instrumentation.DIAGNOSTIC_INFO_KEY]?.[
7375
instrumentation.INSTRUMENTATION_SOURCE_KEY
7476
]?.length,
7577
2
7678
);
7779
assert.equal(
78-
entries[0].data?.[instrumentation.DIAGNOSTIC_INFO_KEY]?.[
80+
entries[0][0].data?.[instrumentation.DIAGNOSTIC_INFO_KEY]?.[
7981
instrumentation.INSTRUMENTATION_SOURCE_KEY
8082
]?.[0]?.[NAME],
8183
instrumentation.NODEJS_LIBRARY_NAME_PREFIX
8284
);
8385
assert.equal(
84-
entries[0].data?.[instrumentation.DIAGNOSTIC_INFO_KEY]?.[
86+
entries[0][0].data?.[instrumentation.DIAGNOSTIC_INFO_KEY]?.[
8587
instrumentation.INSTRUMENTATION_SOURCE_KEY
8688
]?.[1]?.[NAME],
8789
NODEJS_TEST
8890
);
8991
assert.equal(
90-
entries[0].data?.[instrumentation.DIAGNOSTIC_INFO_KEY]?.[
92+
entries[0][0].data?.[instrumentation.DIAGNOSTIC_INFO_KEY]?.[
9193
instrumentation.INSTRUMENTATION_SOURCE_KEY
9294
]?.[1]?.[VERSION],
9395
VERSION_TEST
@@ -97,15 +99,16 @@ describe('instrumentation_info', () => {
9799
it('should replace instrumentation log entry in the list', () => {
98100
const dummyEntry = createEntry('nodejs-test', undefined);
99101
const entries = instrumentation.populateInstrumentationInfo(dummyEntry);
100-
assert.equal(entries.length, 1);
102+
assert.equal(entries[0].length, 1);
103+
assert.equal(true, entries[1]);
101104
assert.equal(
102-
entries[0].data?.[instrumentation.DIAGNOSTIC_INFO_KEY]?.[
105+
entries[0][0].data?.[instrumentation.DIAGNOSTIC_INFO_KEY]?.[
103106
instrumentation.INSTRUMENTATION_SOURCE_KEY
104107
]?.length,
105108
1
106109
);
107110
assert.equal(
108-
entries[0].data?.[instrumentation.DIAGNOSTIC_INFO_KEY]?.[
111+
entries[0][0].data?.[instrumentation.DIAGNOSTIC_INFO_KEY]?.[
109112
instrumentation.INSTRUMENTATION_SOURCE_KEY
110113
]?.[0]?.[NAME],
111114
instrumentation.NODEJS_LIBRARY_NAME_PREFIX
@@ -116,15 +119,16 @@ describe('instrumentation_info', () => {
116119
const entries = instrumentation.populateInstrumentationInfo(
117120
createEntry(LONG_NODEJS_TEST, LONG_VERSION_TEST)
118121
);
119-
assert.equal(entries.length, 1);
122+
assert.equal(entries[0].length, 1);
123+
assert.equal(true, entries[1]);
120124
assert.equal(
121-
entries[0].data?.[instrumentation.DIAGNOSTIC_INFO_KEY]?.[
125+
entries[0][0].data?.[instrumentation.DIAGNOSTIC_INFO_KEY]?.[
122126
instrumentation.INSTRUMENTATION_SOURCE_KEY
123127
]?.[1]?.[NAME],
124128
NODEJS_TEST + '-oo*'
125129
);
126130
assert.equal(
127-
entries[0].data?.[instrumentation.DIAGNOSTIC_INFO_KEY]?.[
131+
entries[0][0].data?.[instrumentation.DIAGNOSTIC_INFO_KEY]?.[
128132
instrumentation.INSTRUMENTATION_SOURCE_KEY
129133
]?.[1]?.[VERSION],
130134
VERSION_TEST + '.0.0.0.0.*'
@@ -134,17 +138,18 @@ describe('instrumentation_info', () => {
134138
it('should add instrumentation log entry only once', () => {
135139
const dummyEntry = createEntry(undefined, undefined);
136140
let entries = instrumentation.populateInstrumentationInfo(dummyEntry);
137-
assert.equal(entries.length, 2);
138-
assert.deepEqual(dummyEntry, entries[0]);
141+
assert.equal(entries[0].length, 2);
142+
assert.deepEqual(dummyEntry, entries[0][0]);
143+
assert.equal(true, entries[1]);
139144
assert.equal(
140-
entries[1].data?.[instrumentation.DIAGNOSTIC_INFO_KEY]?.[
145+
entries[0][1].data?.[instrumentation.DIAGNOSTIC_INFO_KEY]?.[
141146
instrumentation.INSTRUMENTATION_SOURCE_KEY
142147
]?.[0]?.[NAME],
143148
instrumentation.NODEJS_LIBRARY_NAME_PREFIX
144149
);
145150
entries = instrumentation.populateInstrumentationInfo(dummyEntry);
146-
assert.equal(entries.length, 1);
147-
assert.deepEqual(dummyEntry, entries[0]);
151+
assert.equal(entries[0].length, 1);
152+
assert.deepEqual(dummyEntry, entries[0][0]);
148153
});
149154
});
150155

‎test/log.ts‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import {Log as LOG, LogOptions, WriteOptions} from '../src/log';
2222
import {Data, EntryJson, LogEntry} from '../src/entry';
2323

2424
import * as logCommon from '../src/utils/log-common';
25+
import * as instrumentation from '../src/utils/instrumentation';
2526

2627
describe('Log', () => {
2728
let Log: typeof LOG;
@@ -75,6 +76,7 @@ describe('Log', () => {
7576
log.logging.setDetectedResource = () => {
7677
log.logging.detectedResource = FAKE_RESOURCE;
7778
};
79+
instrumentation.setInstrumentationStatus(true);
7880
});
7981

8082
// Create a mock Logging instance
@@ -551,6 +553,31 @@ describe('Log', () => {
551553
)
552554
);
553555
});
556+
557+
it('should set the partialSuccess properly for instrumentation record', async () => {
558+
instrumentation.setInstrumentationStatus(false);
559+
await log.write(ENTRIES, OPTIONS);
560+
assert(
561+
log.logging.loggingService.writeLogEntries.calledOnceWith(
562+
sinon.match({
563+
partialSuccess: true,
564+
})
565+
)
566+
);
567+
instrumentation.setInstrumentationStatus(true);
568+
});
569+
570+
it('should set the partialSuccess properly for existing instrumentation record', async () => {
571+
ENTRIES.push(instrumentation.createDiagnosticEntry(undefined, undefined));
572+
await log.write(ENTRIES, OPTIONS);
573+
assert(
574+
log.logging.loggingService.writeLogEntries.calledOnceWith(
575+
sinon.match({
576+
partialSuccess: true,
577+
})
578+
)
579+
);
580+
});
554581
});
555582

556583
describe('decorateEntries', () => {

0 commit comments

Comments
 (0)