-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(core): Add span.spanContext()
#10037
Conversation
size-limit report 📦
|
3ce59d8
to
25027f9
Compare
packages/core/src/utils/spanUtils.ts
Outdated
export function spanIsSampled(span: Span): boolean { | ||
const { traceFlags } = span.spanContext(); | ||
// eslint-disable-next-line no-bitwise | ||
return Boolean(traceFlags & TraceFlagSampled); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we add a reference to OTel for this bit operation? I assume there's some documentation around the bit fields or masks?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I'll reference where this comes from 😅
packages/core/src/utils/spanUtils.ts
Outdated
export const TraceFlagNone = 0x0; | ||
// eslint-disable-next-line no-bitwise | ||
export const TraceFlagSampled = 0x1 << 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
l: Should these be all uppercase?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good point, makes more sense!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also, 0x1 << 0
is just 0x1
, no?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I think so, I copied this from OTEL, not sure why they have this weird syntax - it is the same, right? 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think so. Maybe they do it just for clarity?
0x1 << 0 // sampled flag
0x1 << 1 // another flag
but yes, it's identical
> 0x1 === 0x1 << 0
true
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I updated it and added a comment that this is aligned to how this is in OTEL!
I get this error after upgrading
|
@mustafa0x can you make sure you have no |
@mydea deleting node_modules and pnpm-lock.yaml did indeed fix! In pnpm-lock.yaml I noticed Thanks! |
This will replace
spanId
,traceId
andsampled
lookups.Because the bitwise format of OTEL is a bit weird IMHO to check if a span is sampled, I also added a utility
spanIsSampled(span)
method to abstract this away.