Update dynamic logging to work with Go's 1.17+ calling convention#2108
Merged
ddelnano merged 5 commits intopixie-io:mainfrom Feb 11, 2025
Merged
Conversation
This change has parity with the previous ABI's implementation with the exception of handling structs that are passed via registers. This will be handled in a later change Signed-off-by: Dom Del Nano <[email protected]>
…OB coverage for struct variables Signed-off-by: Dom Del Nano <[email protected]>
Signed-off-by: Dom Del Nano <[email protected]>
Signed-off-by: Dom Del Nano <[email protected]>
1457947 to
5642386
Compare
ddelnano
added a commit
that referenced
this pull request
Feb 11, 2025
Summary: Upgrade rules_go to v0.47.1 for go 1.22 support The work to remove our dependence on go 1.16 will be complete soon (once #2108 is merged). With that done, we can upgrade our go dependencies and go version. This rules_go change is needed to support go 1.22 as there was an assembler fix in [v0.46.0](https://github.com/bazel-contrib/rules_go/releases/tag/v0.46.0) (bazel-contrib/rules_go#3756 specifically). This was the most recent rules_go version I could upgrade to without dealing with breaking changes. I'll revisit upgrading this to a newer version soon, but my primary goal was to be able to upgrade go and our go deps. Relevant Issues: N/A Type of change: /kind cleanup Test Plan: Existing tests and verified my branch to add go 1.22 as a supported version builds properly Signed-off-by: Dom Del Nano <[email protected]>
oazizi000
reviewed
Feb 11, 2025
Contributor
oazizi000
left a comment
There was a problem hiding this comment.
Should we document somewhere what versions we will support. Should we officially deprecate support for Go 1.16?
src/stirling/source_connectors/dynamic_tracer/dynamic_tracing/dwarvifier.cc
Outdated
Show resolved
Hide resolved
Signed-off-by: Dom Del Nano <[email protected]>
Member
Author
|
@oazizi000 I think that makes sense. I've updated the docs in pixie-io/docs.px.dev#294. That page is linked in Pixie's README and is essentially the documentation for the feature. I'll also post an update on #666 stating that the feature was updated to support Go 1.17+. |
oazizi000
approved these changes
Feb 11, 2025
ddelnano
added a commit
that referenced
this pull request
Feb 11, 2025
…2116) Summary: Add go 1.22 and 1.23 to socket tracer tests. Bump minor go versions This PR adds Go 1.22 and 1.23 to the socket tracer test suite in anticipation of removing Go 1.16 and 1.17 from our repo once #2108 is merged. The plan is to follow this up with a change that removes Go 1.16 and 1.17 in addition to upgrading a portion of our Go dependencies. The bulk of our dependencies still support Go 1.18, so this will allow us to upgrade to the latest versions. Relevant Issues: N/A Type of change: /kind cleanup Test Plan: Test suite passes --------- Signed-off-by: Dom Del Nano <[email protected]>
ddelnano
added a commit
to ddelnano/pixie
that referenced
this pull request
Aug 6, 2025
Summary: Upgrade rules_go to v0.47.1 for go 1.22 support The work to remove our dependence on go 1.16 will be complete soon (once pixie-io#2108 is merged). With that done, we can upgrade our go dependencies and go version. This rules_go change is needed to support go 1.22 as there was an assembler fix in [v0.46.0](https://github.com/bazel-contrib/rules_go/releases/tag/v0.46.0) (bazel-contrib/rules_go#3756 specifically). This was the most recent rules_go version I could upgrade to without dealing with breaking changes. I'll revisit upgrading this to a newer version soon, but my primary goal was to be able to upgrade go and our go deps. Relevant Issues: N/A Type of change: /kind cleanup Test Plan: Existing tests and verified my branch to add go 1.22 as a supported version builds properly Signed-off-by: Dom Del Nano <[email protected]> GitOrigin-RevId: b122242
ddelnano
added a commit
to ddelnano/pixie
that referenced
this pull request
Aug 6, 2025
…xie-io#2108) Summary: Update dynamic logging to work with Go's 1.17+ calling convention Despite our earlier messaging that dynamic logging would be deprecated in pixie-io#2105, I decided that it was easier to handle the new ABI instead of deprecating the Go parts of the dynamic tracer. My rationale for this is that the blocker for achieving feature parity for the new Go ABI, handling `STRUCT_BLOB` variables, is actually a concern for c/c++ tracepoint programs as well. I felt it was important to keep the dynamic tracer rather than deprecate it entirely. As mentioned above, this change does not have full feature parity with the legacy ABI since `STRUCT_BLOB` variables don't work with register based calling conventions -- these variables copy a blob of memory with the assumption that a packed struct exists in a contiguous chunk of memory. However, this isn't unique to Go binaries as c/c++ applications can pass certain structs via [registers](https://github.com/pixie-io/pixie/blob/b0001dab6288508295c54f36c81b3ed80c2677e1/src/stirling/obj_tools/testdata/cc/test_exe.cc#L66-L69) as well. This change replaces the previous Go `STRUCT_BLOB` tests with C equivalents that pass the variable on the stack. When a struct variable will be passed via a register, dynamic logging will now return an error and suggest to the user to access struct fields individually. This error and the test cases that assert the error is returned can be reverted to their original form once pixie-io#2106 is complete. Relevant Issues: pixie-io#2105 Type of change: /kind cleanup Test Plan: Existing tests pass and new ones were added where coverage was needed Changelog Message: Update Go dynamic logging feature to support Go 1.17+ binaries --------- Signed-off-by: Dom Del Nano <[email protected]> GitOrigin-RevId: 8232eee
ddelnano
added a commit
to ddelnano/pixie
that referenced
this pull request
Aug 6, 2025
…ixie-io#2116) Summary: Add go 1.22 and 1.23 to socket tracer tests. Bump minor go versions This PR adds Go 1.22 and 1.23 to the socket tracer test suite in anticipation of removing Go 1.16 and 1.17 from our repo once pixie-io#2108 is merged. The plan is to follow this up with a change that removes Go 1.16 and 1.17 in addition to upgrading a portion of our Go dependencies. The bulk of our dependencies still support Go 1.18, so this will allow us to upgrade to the latest versions. Relevant Issues: N/A Type of change: /kind cleanup Test Plan: Test suite passes --------- Signed-off-by: Dom Del Nano <[email protected]> GitOrigin-RevId: 09ade3b
ddelnano
added a commit
to k8sstormcenter/pixie
that referenced
this pull request
Feb 25, 2026
Summary: Upgrade rules_go to v0.47.1 for go 1.22 support The work to remove our dependence on go 1.16 will be complete soon (once pixie-io#2108 is merged). With that done, we can upgrade our go dependencies and go version. This rules_go change is needed to support go 1.22 as there was an assembler fix in [v0.46.0](https://github.com/bazel-contrib/rules_go/releases/tag/v0.46.0) (bazel-contrib/rules_go#3756 specifically). This was the most recent rules_go version I could upgrade to without dealing with breaking changes. I'll revisit upgrading this to a newer version soon, but my primary goal was to be able to upgrade go and our go deps. Relevant Issues: N/A Type of change: /kind cleanup Test Plan: Existing tests and verified my branch to add go 1.22 as a supported version builds properly Signed-off-by: Dom Del Nano <[email protected]>
ddelnano
added a commit
to k8sstormcenter/pixie
that referenced
this pull request
Feb 25, 2026
…xie-io#2108) Summary: Update dynamic logging to work with Go's 1.17+ calling convention Despite our earlier messaging that dynamic logging would be deprecated in pixie-io#2105, I decided that it was easier to handle the new ABI instead of deprecating the Go parts of the dynamic tracer. My rationale for this is that the blocker for achieving feature parity for the new Go ABI, handling `STRUCT_BLOB` variables, is actually a concern for c/c++ tracepoint programs as well. I felt it was important to keep the dynamic tracer rather than deprecate it entirely. As mentioned above, this change does not have full feature parity with the legacy ABI since `STRUCT_BLOB` variables don't work with register based calling conventions -- these variables copy a blob of memory with the assumption that a packed struct exists in a contiguous chunk of memory. However, this isn't unique to Go binaries as c/c++ applications can pass certain structs via [registers](https://github.com/pixie-io/pixie/blob/b0001dab6288508295c54f36c81b3ed80c2677e1/src/stirling/obj_tools/testdata/cc/test_exe.cc#L66-L69) as well. This change replaces the previous Go `STRUCT_BLOB` tests with C equivalents that pass the variable on the stack. When a struct variable will be passed via a register, dynamic logging will now return an error and suggest to the user to access struct fields individually. This error and the test cases that assert the error is returned can be reverted to their original form once pixie-io#2106 is complete. Relevant Issues: pixie-io#2105 Type of change: /kind cleanup Test Plan: Existing tests pass and new ones were added where coverage was needed Changelog Message: Update Go dynamic logging feature to support Go 1.17+ binaries --------- Signed-off-by: Dom Del Nano <[email protected]>
ddelnano
added a commit
to k8sstormcenter/pixie
that referenced
this pull request
Feb 25, 2026
…ixie-io#2116) Summary: Add go 1.22 and 1.23 to socket tracer tests. Bump minor go versions This PR adds Go 1.22 and 1.23 to the socket tracer test suite in anticipation of removing Go 1.16 and 1.17 from our repo once pixie-io#2108 is merged. The plan is to follow this up with a change that removes Go 1.16 and 1.17 in addition to upgrading a portion of our Go dependencies. The bulk of our dependencies still support Go 1.18, so this will allow us to upgrade to the latest versions. Relevant Issues: N/A Type of change: /kind cleanup Test Plan: Test suite passes --------- Signed-off-by: Dom Del Nano <[email protected]>
ddelnano
added a commit
to k8sstormcenter/pixie
that referenced
this pull request
Feb 25, 2026
Summary: Upgrade rules_go to v0.47.1 for go 1.22 support The work to remove our dependence on go 1.16 will be complete soon (once pixie-io#2108 is merged). With that done, we can upgrade our go dependencies and go version. This rules_go change is needed to support go 1.22 as there was an assembler fix in [v0.46.0](https://github.com/bazel-contrib/rules_go/releases/tag/v0.46.0) (bazel-contrib/rules_go#3756 specifically). This was the most recent rules_go version I could upgrade to without dealing with breaking changes. I'll revisit upgrading this to a newer version soon, but my primary goal was to be able to upgrade go and our go deps. Relevant Issues: N/A Type of change: /kind cleanup Test Plan: Existing tests and verified my branch to add go 1.22 as a supported version builds properly Signed-off-by: Dom Del Nano <[email protected]>
ddelnano
added a commit
to k8sstormcenter/pixie
that referenced
this pull request
Feb 25, 2026
…xie-io#2108) Summary: Update dynamic logging to work with Go's 1.17+ calling convention Despite our earlier messaging that dynamic logging would be deprecated in pixie-io#2105, I decided that it was easier to handle the new ABI instead of deprecating the Go parts of the dynamic tracer. My rationale for this is that the blocker for achieving feature parity for the new Go ABI, handling `STRUCT_BLOB` variables, is actually a concern for c/c++ tracepoint programs as well. I felt it was important to keep the dynamic tracer rather than deprecate it entirely. As mentioned above, this change does not have full feature parity with the legacy ABI since `STRUCT_BLOB` variables don't work with register based calling conventions -- these variables copy a blob of memory with the assumption that a packed struct exists in a contiguous chunk of memory. However, this isn't unique to Go binaries as c/c++ applications can pass certain structs via [registers](https://github.com/pixie-io/pixie/blob/b0001dab6288508295c54f36c81b3ed80c2677e1/src/stirling/obj_tools/testdata/cc/test_exe.cc#L66-L69) as well. This change replaces the previous Go `STRUCT_BLOB` tests with C equivalents that pass the variable on the stack. When a struct variable will be passed via a register, dynamic logging will now return an error and suggest to the user to access struct fields individually. This error and the test cases that assert the error is returned can be reverted to their original form once pixie-io#2106 is complete. Relevant Issues: pixie-io#2105 Type of change: /kind cleanup Test Plan: Existing tests pass and new ones were added where coverage was needed Changelog Message: Update Go dynamic logging feature to support Go 1.17+ binaries --------- Signed-off-by: Dom Del Nano <[email protected]>
ddelnano
added a commit
to k8sstormcenter/pixie
that referenced
this pull request
Feb 25, 2026
…ixie-io#2116) Summary: Add go 1.22 and 1.23 to socket tracer tests. Bump minor go versions This PR adds Go 1.22 and 1.23 to the socket tracer test suite in anticipation of removing Go 1.16 and 1.17 from our repo once pixie-io#2108 is merged. The plan is to follow this up with a change that removes Go 1.16 and 1.17 in addition to upgrading a portion of our Go dependencies. The bulk of our dependencies still support Go 1.18, so this will allow us to upgrade to the latest versions. Relevant Issues: N/A Type of change: /kind cleanup Test Plan: Test suite passes --------- Signed-off-by: Dom Del Nano <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary: Update dynamic logging to work with Go's 1.17+ calling convention
Despite our earlier messaging that dynamic logging would be deprecated in #2105, I decided that it was easier to handle the new ABI instead of deprecating the Go parts of the dynamic tracer. My rationale for this is that the blocker for achieving feature parity for the new Go ABI, handling
STRUCT_BLOBvariables, is actually a concern for c/c++ tracepoint programs as well. I felt it was important to keep the dynamic tracer rather than deprecate it entirely.As mentioned above, this change does not have full feature parity with the legacy ABI since
STRUCT_BLOBvariables don't work with register based calling conventions -- these variables copy a blob of memory with the assumption that a packed struct exists in a contiguous chunk of memory. However, this isn't unique to Go binaries as c/c++ applications can pass certain structs via registers as well.This change replaces the previous Go
STRUCT_BLOBtests with C equivalents that pass the variable on the stack. When a struct variable will be passed via a register, dynamic logging will now return an error and suggest to the user to access struct fields individually. This error and the test cases that assert the error is returned can be reverted to their original form once #2106 is complete.Relevant Issues: #2105
Type of change: /kind cleanup
Test Plan: Existing tests pass and new ones were added where coverage was needed
Changelog Message: Update Go dynamic logging feature to support Go 1.17+ binaries