Skip to content
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

Add pad_start link formatting function #2505 #2504

Merged

Conversation

drewcorlin1
Copy link
Contributor

@drewcorlin1 drewcorlin1 commented Nov 30, 2024

Resolves #2505

Given a traceparent header like 00-00000000000000001b3ef83a5d15ed1b-61409d351b3a5d99-01, jaeger will store (and thus display + interpolate) the trace ID as 1b3ef83a5d15ed1b. When attempting to use the shortened trace ID to link to logs some tracing integrations (in my case, logs using pino instrumented with the OpenTelemetry pino instrumentation) will use the full length trace ID, making jaeger link patterns not work.

This PR adds a pad_start formatting function, heavily inspired by javascript's String.padStart to format the trace ID in those cases. If approved, I'll update the documentation in https://github.com/jaegertracing/documentation to mention this function and it's behavior.

In addition to this specific function, I'd like feedback on the approach of passing arguments to formatting functions with the space-delimited approach (eg #{traceID | padStart 32 0}).

How was this change tested?

Unit tests and by running it locally with the following UI config

const DEFAULT_CONFIG = {
  "linkPatterns": [
    {
      "type": "traces",
      "url": "https://myOpenSearch.com/_dashboards/app/discover#/?_g=(time:(from:'#{startTime | epoch_micros_to_date_iso}',to:'#{endTime | epoch_micros_to_date_iso}'))&_a=(columns:!(_source),filters:!(('$state':(store:appState),meta:(alias:!n,disabled:!f,index:filebeat-all,key:json.traceId,negate:!f,params:(query:'#{traceID | pad_start 32 0}'),type:phrase),query:(match_phrase:(json.traceId:'#{traceID | pad_start 32 0}')))),index:filebeat-all)",
      "text": "Logs"
    }
  ]
};

@drewcorlin1 drewcorlin1 requested a review from a team as a code owner November 30, 2024 19:06
@drewcorlin1 drewcorlin1 requested review from joe-elliott and removed request for a team November 30, 2024 19:06
@drewcorlin1 drewcorlin1 force-pushed the pad-start-formatting-function branch from c76ecff to 1234b1d Compare November 30, 2024 19:09
Copy link

codecov bot commented Nov 30, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.62%. Comparing base (d9315c6) to head (97069e3).
Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2504   +/-   ##
=======================================
  Coverage   96.62%   96.62%           
=======================================
  Files         255      255           
  Lines        7697     7707   +10     
  Branches     2003     1990   -13     
=======================================
+ Hits         7437     7447   +10     
  Misses        260      260           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@drewcorlin1 drewcorlin1 changed the title Pad start formatting function Pad start formatting function #2505 Nov 30, 2024
Copy link
Member

@yurishkuro yurishkuro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please add tests for edge cases

@drewcorlin1 drewcorlin1 force-pushed the pad-start-formatting-function branch from 1234b1d to e1ff18e Compare November 30, 2024 19:20
@drewcorlin1
Copy link
Contributor Author

Add for this and for epoch_micros_to_date_iso

@drewcorlin1 drewcorlin1 force-pushed the pad-start-formatting-function branch 2 times, most recently from 6cae02a to a41b7da Compare November 30, 2024 19:30
@drewcorlin1 drewcorlin1 force-pushed the pad-start-formatting-function branch from a41b7da to 97069e3 Compare November 30, 2024 20:27
@yurishkuro yurishkuro changed the title Pad start formatting function #2505 Add pad_start link formatting function #2505 Dec 1, 2024
@yurishkuro yurishkuro added the changelog:bugfix-or-minor-feature 🐞 Bug fixes, Minor Improvements label Dec 1, 2024
@yurishkuro yurishkuro merged commit a083195 into jaegertracing:main Dec 1, 2024
8 of 9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog:bugfix-or-minor-feature 🐞 Bug fixes, Minor Improvements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature]: Add pad start formatting function
2 participants