- 
                Notifications
    
You must be signed in to change notification settings  - Fork 50
 
[RUM-1830] 128 bit trace IDs #680
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
[RUM-1830] 128 bit trace IDs #680
Conversation
6747dd4    to
    2fa4a82      
    Compare
  
            
          
                ...ages/core/src/rum/instrumentation/resourceTracking/distributedTracing/distributedTracing.tsx
              
                Outdated
          
            Show resolved
            Hide resolved
        
              
          
                ...rum/instrumentation/resourceTracking/distributedTracing/__tests__/distributedTracing.test.ts
              
                Outdated
          
            Show resolved
            Hide resolved
        
              
          
                ...rum/instrumentation/resourceTracking/distributedTracing/__tests__/distributedTracing.test.ts
              
                Outdated
          
            Show resolved
            Hide resolved
        
              
          
                ...rum/instrumentation/resourceTracking/distributedTracing/__tests__/distributedTracing.test.ts
              
                Outdated
          
            Show resolved
            Hide resolved
        
              
          
                ...rum/instrumentation/resourceTracking/distributedTracing/__tests__/distributedTracing.test.ts
              
                Outdated
          
            Show resolved
            Hide resolved
        
              
          
                ...ages/core/src/rum/instrumentation/resourceTracking/distributedTracing/distributedTracing.tsx
              
                Outdated
          
            Show resolved
            Hide resolved
        
      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.
Some suggestions, overall on right track.
One thing that I couldn't see how we are dealing with RUM, Trace or Log events with this update. I assume some encode/decode to be changed.
        
          
                ...rum/instrumentation/resourceTracking/distributedTracing/__tests__/distributedTracing.test.ts
              
                Outdated
          
            Show resolved
            Hide resolved
        
              
          
                ...ages/core/src/rum/instrumentation/resourceTracking/distributedTracing/distributedTracing.tsx
              
                Outdated
          
            Show resolved
            Hide resolved
        
              
          
                ...ages/core/src/rum/instrumentation/resourceTracking/distributedTracing/distributedTracing.tsx
              
                Outdated
          
            Show resolved
            Hide resolved
        
              
          
                ...ages/core/src/rum/instrumentation/resourceTracking/distributedTracing/distributedTracing.tsx
              
                Outdated
          
            Show resolved
            Hide resolved
        
              
          
                ...ages/core/src/rum/instrumentation/resourceTracking/distributedTracing/distributedTracing.tsx
              
                Outdated
          
            Show resolved
            Hide resolved
        
      7319839    to
    4be127d      
    Compare
  
    | 
           I have changed the implementation, here is how it works now: Separating Span ID and Trace IDThe  It can only be created through  Span IDs will always be generated as 64 bits BigInt(s) and Trace IDs as 128 bits BigInt(s) String representation takes into account the type of Tracing IdentifierTracingIdFormat (previously  When requesting a padded HEX value, the bit length of the current type will be taken into account. Requesting the low bits of a 128 bit Trace ID will get you a 64 bit ID, while requesting the low bits of a 64 bit Span ID will get you a 32 bit ID. Here is a runnable example. Here is an example output for reference: 
 I will add a few more tests to match the RFC specs more accurately, and verify @ganeshnj's comment #680 (review).  | 
    
a069027    to
    1995e82      
    Compare
  
    | 
           Example of generated headers: 
  | 
    
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.
Nice updates, just a few improvement comments but looks good to me.
        
          
                packages/core/src/rum/instrumentation/resourceTracking/distributedTracing/TracingIdentifier.tsx
              
                Outdated
          
            Show resolved
            Hide resolved
        
              
          
                ...rum/instrumentation/resourceTracking/distributedTracing/__tests__/distributedTracing.test.ts
              
                Outdated
          
            Show resolved
            Hide resolved
        
      26daf7c    to
    aade66d      
    Compare
  
    aade66d    to
    85e8440      
    Compare
  
    
What does this PR do?
Introduces 128 bit trace IDs.
Review checklist (to be filled by reviewers)