chore: Update CONTRIBUTING with process and minor changes#255
chore: Update CONTRIBUTING with process and minor changes#255
Conversation
Lms24
requested review from
a team,
cleptric,
lcian,
mjq and
nsdeschenes
as code owners
Semver Impact of This PR🟢 Patch (bug fixes) 📋 Changelog PreviewThis is how your changes will appear in the changelog. New Features ✨Attributes
Other
Bug Fixes 🐛Attributes
Other
Documentation 📚
Internal Changes 🔧Attributes
Deps Dev
Repo
Other
Other
🤖 This preview updates automatically when you update the PR. |
|
Gonna put this to draft for now since things are moving fast and we'll prioritize getting conventions up to speed before we make the process stricter. |
|
|
||
| 1. Open a PR with the proposed convention ([Adding an Attribute](#adding-a-new-attribute)). | ||
| 2. Get an approval from at least one code owner. | ||
| 3. Wait for at least 3 business days after the first approval to give other code owners a chance to review. |
There was a problem hiding this comment.
imho we should explain somewhere why the 3 day waiting period, something along the lines of
"grace period exists because attribute names, once shipped in an SDK release, are effectively permanent. If a bad name gets adopted by even one SDK, fixing it requires a deprecation cycle across every SDK that shipped it."
we could make it even more stark: "There is no urgency exception if a name feels wrong, raise it during review, not after. "
|
|
||
| 1. Open a PR with the proposed convention ([Adding an Attribute](#adding-a-new-attribute)). | ||
| 2. Get an approval from at least one code owner. | ||
| 3. Wait for at least 3 business days after the first approval to give other code owners a chance to review. |
There was a problem hiding this comment.
who is gonna be code owner here?
@stephanie-anderson @getsentry/sdk-seniors ?
| 3. Wait for at least 3 business days after the first approval to give other code owners a chance to review. | ||
| 4. Merge your PR (alternatively, code owners may merge it after review) | ||
|
|
||
| ## Adding a new attribute |
There was a problem hiding this comment.
I suggest to add a paragraph about OTel alignment here. It could look something like this:
OTel Alignment
Before proposing a new attribute, check the OpenTelemetry semantic conventions registry.
If OTel already defines the attribute:
- Use the OTel name and type. Set
is_in_otel: true. - Do not create a Sentry-specific synonym!. Diverging from OTel for the same concept creates mapping work for every integration.
If OTel doesn't define it, or the concept is Sentry-specific, set is_in_otel: false.
When in doubt, prefer OTel alignment. A Sentry-specific name can be deprecated later; an OTel-aligned name is forward-compatible by default.
@Lms24 does that make sense?
| ## Policies | ||
|
|
||
| ## Attributes | ||
| ### Attributes |
There was a problem hiding this comment.
should we add naming rules & examples?
Naming rules:
- Use dots as separators, not underscores (
http.request.method, nothttp_request_method) - Namespace first (
db.system, notsystem.db) - Keep names stable! Renames require deprecation cycles across all SDKs that adopted the attribute!
dingsdax
left a comment
dingsdax
left a comment
There was a problem hiding this comment.
Do we want something about the generates typed constants in CONTRIBUTING.md? What's our plan there, do we encourage that?
4 participants
Uh oh!
There was an error while loading. Please reload this page.