Skip to content

SK-246 // transform trace on export via a SKEL file#252

Open
ogorman89 wants to merge 6 commits into
mainfrom
ian/export-transform-flag
Open

SK-246 // transform trace on export via a SKEL file#252
ogorman89 wants to merge 6 commits into
mainfrom
ian/export-transform-flag

Conversation

@ogorman89
Copy link
Copy Markdown
Contributor

@ogorman89 ogorman89 commented May 26, 2026

Description and Rationale

feat: transform trace on export via a SKEL file

How

  • add transform flag to export in CLI --transform <file.skel>
  • modify the export command to apply the SKEL transformation
  • in order to re-use our existing process_trace and apply_skel functionality I moved them out of sk-store and into sk-core, this reorganization makes the number of file changes in this PR quite large (apologies)
  • updated apply_skel to take a SKEL string (instead of a SKEL file path), primarily this change is to support easily passing the SKEL transformation to the tracer.

Test Steps

  • automated testing
  • manual testing by exporting small traces from a local cluster with and without SKEL transformations

Other Notes

  • followups & questions:

    • behavior of large trace file export/transformation still needs to be properly tested
    • I think it may make sense to implement the same logic for snapshot?
    • right now we collect all the events then loop through the events a second time to transform them, there are likely efficiencies to be extracted here but probably outside the scope of this PR
    • should this transform give a nice loading bar like skctl transform?
  • [ X ] I certify that this PR does not contain any code that has been generated with GitHub Copilot or any other AI-based code generation tool, in accordance with this project's policies.

@linear
Copy link
Copy Markdown

linear Bot commented May 26, 2026

SK-246

@ogorman89 ogorman89 requested a review from drmorr0 May 26, 2026 17:51
@ogorman89 ogorman89 self-assigned this May 26, 2026
@codecov
Copy link
Copy Markdown

codecov Bot commented May 26, 2026

Codecov Report

❌ Patch coverage is 40.00000% with 54 lines in your changes missing coverage. Please review.
✅ Project coverage is 77.95%. Comparing base (05ae6be) to head (699cad3).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
sk-store/src/process.rs 42.50% 23 Missing ⚠️
sk-store/src/store.rs 55.00% 9 Missing ⚠️
sk-cli/src/export.rs 0.00% 8 Missing ⚠️
sk-cli/src/snapshot.rs 0.00% 6 Missing ⚠️
sk-core/src/event.rs 0.00% 6 Missing ⚠️
sk-cli/src/transform.rs 0.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #252      +/-   ##
==========================================
- Coverage   78.37%   77.95%   -0.42%     
==========================================
  Files          62       63       +1     
  Lines        3852     3883      +31     
==========================================
+ Hits         3019     3027       +8     
- Misses        833      856      +23     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Comment thread sk-cli/src/snapshot.rs
@@ -42,7 +42,12 @@ pub async fn cmd(args: &Args) -> EmptyResult {
let filters = ExportFilters::new(args.excluded_namespaces.clone(), vec![]);
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

should snapshot have the same --transform option as export?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant