feat(gengapic): add dynamic resource name heuristics#1722
feat(gengapic): add dynamic resource name heuristics#1722quartzmo merged 23 commits intogoogleapis:mainfrom
Conversation
… name fields Migrate resourceNameField from a single string accessor to a HeuristicTarget struct (containing a format string and field names). - Why: Standard field getters fail for unannotated legacy paths with multiple variables (e.g., /projects/%s/locations/%s). - Host Resolution: Resolves the service DefaultHost extension at generator-time instead of runtime to evaluate paths (such as //foo.googleapis.com/%v) without passing host into fmt.Sprintf dynamically.
Implement dynamic resource name heuristics to infer resource name formatting for unannotated services. - Add BuildHeuristicVocabulary to learn valid collection nouns from standard methods - Add IdentifyHeuristicTarget to walk backward and infer resource name templates - Strip custom verbs (e.g. :cancel, :publish) from literals before checking vocabulary - Add test cases for interstitial literals (e.g. global in Compute Engine) - Add test coverage for multiple variables and disconnected chains
…ntext enable F_open_telemetry_tracing F_open_telemetry_logging and F_open_telemetry_metrics in showcase testing
westarle
left a comment
westarle
left a comment
There was a problem hiding this comment.
This is looking good, I think we can just tidy things up a little bit by collecting everything in injectTelemetryContext and fixing keys.
westarle
left a comment
westarle
left a comment
There was a problem hiding this comment.
I think we at least need to fix the keys so it works, tidy up can happen in another PR.
shollyman
left a comment
shollyman
left a comment
There was a problem hiding this comment.
How much do you care about independent feature enablement at this point? Your implementation looks to be basically OR-checking that any of the three o11y generator features is enabled and emitting shared code (which is then emitting ifFeatureEnabled OR-ing). If they're not easily decoupled just collapse them into a single generator feature.
Similarly, I see you've defined DynamicResourceHeuristicsFeature here and use it in gengapic but I see no tests that turn it on, nor anything that compares what happens with it on or off.
Yeah, I think we weren't sure about what data was required for each signal before. It makes sense to consolidate the generator side flags to We must keep IsFeatureEnabled("TRACING") tests independent. Of course we can |
shollyman
left a comment
shollyman
left a comment
There was a problem hiding this comment.
There's a lot going on here, but I think its LGTM. The unit testing exercises the heuristic, but we don't appear to have any golden output tests that demonstrate the heuristic in practice. My assumption this is coming in a followup.
3 participants
This PR implements fallback heuristics to deduce resource name templates for unannotated legacy services (like Compute Engine).
It is gated behind a new DynamicResourceHeuristicsFeature flag.
The solution is a two-step process:
The learned vocabulary is stored as a new field on the Generator struct so it can be shared across file generation runs without redundant rescans. The implementation includes robust custom verb stripping (e.g. :cancel, :publish), correctly handles interstitial literals like "global", and discards partial matches if variables sitting to the left are not in the vocabulary.