Skip to content

fix(tool): support path helpers for call rules#482

Open
ecleon26 wants to merge 1 commit into
open-telemetry:mainfrom
ecleon26:fix-call-rule-path-helpers
Open

fix(tool): support path helpers for call rules#482
ecleon26 wants to merge 1 commit into
open-telemetry:mainfrom
ecleon26:fix-call-rule-path-helpers

Conversation

@ecleon26
Copy link
Copy Markdown

Description

Adds path support to call rules so replacement wrappers can reference helper functions that live outside the target source file.

This change:

  • adds Path parsing to InstCallRule
  • detects unqualified helper calls used by call-rule replace / append_args
  • materializes matching helper files from the configured path into the target package during instrumentation
  • updates importcfg for helper-file imports
  • updates the affected golden fixtures, including wrap_sizeof_call and wrap_sizeof_with_strings
  • documents the new call-rule path behavior

Motivation

Call-rule YAML fixtures can specify wrapper expressions such as Wrapper({{ . }}), but InstCallRule previously had no Path field. As a result, adding path: to YAML would be silently ignored, and call-rule replacements could only reference helpers already defined in the target source file or symbols available through imports.

This fixes the missing code path so helper functions can be located and compiled into the target package when a call rule needs them.

Fixes #


Checklist

  • PR title follows conventional commits format
  • Code formatted: make format
  • Linters pass: make lint
  • Tests pass: make test
  • Tests added for new functionality
  • Tests follow testing guidelines
  • Documentation updated (if applicable)

Add path parsing and helper file materialization for call rules so wrapper replacements can reference helper functions outside the target source file.

Assisted-by: ChatGPT 5
@ecleon26 ecleon26 requested a review from a team as a code owner May 10, 2026 08:13
@linux-foundation-easycla
Copy link
Copy Markdown

linux-foundation-easycla Bot commented May 10, 2026

CLA Signed
The committers listed above are authorized under a signed CLA.

  • ✅ login: ecleon26 / name: narrative26 (d7c1961)

@github-actions github-actions Bot added the scope:fix A bug that is being fixed label May 10, 2026
@ecleon26
Copy link
Copy Markdown
Author

@y1yang0

@amazingakai
Copy link
Copy Markdown
Contributor

Thank you for the contribution.

We already support helper usage through the existing imports field. For example, if we want to use our own wrapper/helper code around a function, we can define it inside github.com/open-telemetry/opentelemetry-go-compile-instrumentation/pkg/instrumentation/..., add it through imports, and then reference it directly inside replace.
From my understanding, the main missing piece right now is support for this in golden tests, since we would need compileArgs/createImportCfg to compile and resolve those helper imports correctly there as well (please see my comment on the original issue).

@ecleon26
Copy link
Copy Markdown
Author

That makes sense to me. I agree that broadening path to call/raw/decl rules may blur the existing rule semantics, since those rules already have an imports mechanism for external helper code.

For call rules specifically, it seems cleaner to reference helpers through an imported package alias in replace, for example wrapper.Wrapper({{ . }}), and declare that package through imports. That keeps helper resolution explicit and consistent with raw/decl-style expression injection.

So I think the better fix may be limited to golden-test infrastructure: either teach compileArgs / createImportCfg how to resolve testcase-local helper packages, or avoid testcase-local wrapper imports if that adds too much test-only complexity. I do not think we should add path support across more rule types unless we want a broader rule-model change.

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

Labels

scope:fix A bug that is being fixed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants