feat(generator): add internalTelemetryInfo configuration to nunjucks templates - #9174
feat(generator): add internalTelemetryInfo configuration to nunjucks templates#9174shivanee-p wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces telemetry tracing configuration (internalTelemetryInfo) to the generated service client templates for both CommonJS and ESM formats when telemetry tracing is enabled. The review feedback highlights a critical issue where directly mutating the opts object can lead to a TypeError if opts is undefined, or cause unintended side-effects if the options object is reused. It is recommended to safely clone opts using Object.assign({}, opts) before assigning the telemetry properties.
| {%- if api.enableTelemetryTracing %} | ||
| opts.internalTelemetryInfo = { | ||
| gcpClientService: '{{ api.loggingName }}', | ||
| gcpClientVersion: '{{ api.naming.version }}', | ||
| gcpRepo: 'googleapis/google-cloud-node', | ||
| gcpArtifact: '{{ api.publishName }}', | ||
| } | ||
| {%- endif %} |
There was a problem hiding this comment.
Directly mutating the opts object passed to the constructor is unsafe and can lead to unexpected side-effects if the user reuses the options object across multiple clients. Additionally, since opts is optional (opts?: ClientOptions), attempting to set properties on it directly will throw a TypeError at runtime if the client is instantiated without options.
To prevent both issues, create a shallow copy of opts using Object.assign({}, opts) before assigning the telemetry info. This safely handles undefined (resulting in an empty object {}) and protects the user's original configuration from mutation.
{%- if api.enableTelemetryTracing %}
opts = Object.assign({}, opts);
opts.internalTelemetryInfo = {
gcpClientService: '{{ api.loggingName }}',
gcpClientVersion: '{{ api.naming.version }}',
gcpRepo: 'googleapis/google-cloud-node',
gcpArtifact: '{{ api.publishName }}',
};
{%- endif %}
| {%- if api.enableTelemetryTracing %} | ||
| opts.internalTelemetryInfo = { | ||
| gcpClientService: '{{ api.loggingName }}', | ||
| gcpClientVersion: '{{ api.naming.version }}', | ||
| gcpRepo: 'googleapis/google-cloud-node', | ||
| gcpArtifact: '{{ api.publishName }}', | ||
| } | ||
| {%- endif %} |
There was a problem hiding this comment.
Directly mutating the opts object passed to the constructor is unsafe and can lead to unexpected side-effects if the user reuses the options object across multiple clients. Additionally, since opts is optional (opts?: ClientOptions), attempting to set properties on it directly will throw a TypeError at runtime if the client is instantiated without options.
To prevent both issues, create a shallow copy of opts using Object.assign({}, opts) before assigning the telemetry info. This safely handles undefined (resulting in an empty object {}) and protects the user's original configuration from mutation.
{%- if api.enableTelemetryTracing %}
opts = Object.assign({}, opts);
opts.internalTelemetryInfo = {
gcpClientService: '{{ api.loggingName }}',
gcpClientVersion: '{{ api.naming.version }}',
gcpRepo: 'googleapis/google-cloud-node',
gcpArtifact: '{{ api.publishName }}',
};
{%- endif %}
33674b2 to
f8305b2
Compare
f8305b2 to
c13d658
Compare
c13d658 to
feea2ed
Compare
a263692 to
d9982c7
Compare
d9982c7 to
249eea0
Compare
249eea0 to
07e057a
Compare
07e057a to
93e864d
Compare
93e864d to
0835dbf
Compare
0835dbf to
9077df6
Compare
c94c338 to
3069cba
Compare
3069cba to
fab49c6
Compare
d2c3909 to
1f31ff8
Compare
1f31ff8 to
3405ce7
Compare
3405ce7 to
4011f58
Compare
4011f58 to
7eb85b0
Compare
7eb85b0 to
df8d5df
Compare
df8d5df to
0150199
Compare
…elemetryTracing is enabled
0150199 to
a5a0313
Compare
No description provided.