Skip to content

feat(o11y): introduce observability flags needed within the generator - #9052

Open
shivanee-p wants to merge 2 commits into
mainfrom
shivaneep-o11y-flag
Open

feat(o11y): introduce observability flags needed within the generator#9052
shivanee-p wants to merge 2 commits into
mainfrom
shivaneep-o11y-flag

Conversation

@shivanee-p

Copy link
Copy Markdown
Contributor

Adds support for the o11y feature flag to gapic-generator-typescript and updates client options interface definitions in google-gax

  • Added enableTelemetryTracing flag parsing (enable_telemetry_tracing=true)
  • Added enableTelemetryTracing option support to API schema and Naming options
  • Added optional fields to ClientOptions interface
  • Added InternalTelemetry interface definition
  • Added corresponding unit tests

@shivanee-p
shivanee-p requested a review from a team as a code owner July 31, 2026 20:38
@github-actions
github-actions Bot requested a review from feywind July 31, 2026 20:38

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces support for an enableTelemetryTracing option across the generator, schema, and client interface packages, including adding the option to ClientOptions and introducing an InternalTelemetry interface. Feedback suggests updating the parameter parsing in the generator to support both hyphenated and snake_case parameter names for consistency with other flags, and to only set the property to true when explicitly configured.

Comment on lines +231 to +233
private readEnableTelemetryTracing() {
this.enableTelemetryTracing = this.paramMap['enable_telemetry_tracing'] === 'true';
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

To maintain consistency with other parameter parsing methods in this class (such as readLegacyProtoLoad), we should only set the property to true if the parameter is explicitly provided as 'true'. Additionally, we should support the hyphenated parameter name enable-telemetry-tracing to match the repository's naming convention for generator flags.

  private readEnableTelemetryTracing() {
    if (this.paramMap['enable-telemetry-tracing'] === 'true' || this.paramMap['enable_telemetry_tracing'] === 'true') {
      this.enableTelemetryTracing = true;
    }
  }

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the way that flags are read in and manipulated from the .yaml file aren't consistent depending on what level of flag it is. Because this will end up being the name of an attribute within the extra_protoc_parameters field, it wouldn't get changed to kebab case, so it will remain with the underscores.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant