feat(names): Add function span name and description rules - #618
Conversation
Semver Impact of This PR🟡 Minor (new features) 📋 Changelog PreviewThis is how your changes will appear in the changelog. New Features ✨Attributes
Names
Other
Bug Fixes 🐛
Internal Changes 🔧Deps
Other
🤖 This preview updates automatically when you update the PR. |
|
bugbot review |
1 similar comment
|
bugbot review |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 6c5cd17. Configure here.
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| { | ||
| "name": "Function execution", | ||
| "brief": "The execution of a function or a set of instructions that isn't covered by a more specific operation.", | ||
| "ops": ["function"], |
There was a problem hiding this comment.
Bug: The system lacks validation to ensure that ops referenced in name/description files (e.g., function.json) actually exist, creating a risk of future inconsistencies if ops are modified.
Severity: LOW
Suggested Fix
Enhance the test suite (test/name.test.ts and test/description.test.ts) to validate that all strings listed in the ops array of name and description files correspond to an existing operation defined in one of the model/op/*.json category files. This will enforce referential integrity and prevent future breakages.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: model/description/function.json#L7
Potential issue: The schemas for name and description files, `name.schema.json` and
`description.schema.json`, define the `ops` field as an array of strings without any
validation that these strings correspond to actual operations defined in `model/op/`.
While the `function` op referenced in the new files does exist in
`model/op/general.json`, the system lacks a test to enforce this referential integrity.
This creates a potential for future inconsistencies, where an op could be renamed or
removed from the `model/op/` files, but the corresponding name/description files would
not be flagged as invalid by the test suite, leading to broken behavior for those
operations.
Also affects:
model/name/function.json:8
Did we get this right? 👍 / 👎 to inform future reviews.
There was a problem hiding this comment.
good point. will address in a follow-up!
| { | ||
| "name": "Function execution", | ||
| "brief": "The execution of a function or a set of instructions that isn't covered by a more specific operation.", | ||
| "ops": ["function"], |
There was a problem hiding this comment.
Bug: The new generic "function" naming rules will unintentionally apply to more specific sub-ops like "function.gcp" and "function.nextjs" due to prefix matching.
Severity: LOW
Suggested Fix
To prevent unintended inheritance, consider adding specific (even if empty) naming and description rules for each of the function.* sub-ops. Alternatively, if this fallback behavior is desired, document this interaction in the PR description to confirm it is intentional.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: model/description/function.json#L7
Potential issue: The new generic naming and description rules for the `"function"`
operation will be inherited by more specific sub-operations like `"function.gcp"`,
`"function.aws"`, and `"function.nextjs"`. This is due to a prefix-matching mechanism
where specific ops inherit rules from their parent if no specific rules exist for them.
The pull request does not acknowledge this inheritance, suggesting it may be an
unintended side effect. This could lead to these more specific function spans receiving
generic names instead of potentially more appropriate, specific ones.
Also affects:
model/name/function.json:8
2 participants
Description
Adds name and description templates for
functionspans, used to group code function invocations. I propose for both, name and description, we rely on the{code.function.name}attribute which is what our SDKs set today mostly. Should be low card. enough.ref getsentry/sentry-javascript#23954
PR Checklist
yarn testand verified that the tests pass.yarn generateto generate and format code and docs.If an attribute was added:
nextjs.function_id, notfunction_id)apply_scrubbing(i.e.manualorauto. Useneveronly for values that should never be scrubbed such as IDs)If an attribute was deprecated: