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
…elemetryTracing is enabled
0835dbf to
9077df6
Compare
No description provided.