Skip to content

Forward telemetry config to kernel backend - #451

Open
jay-xiao446 wants to merge 1 commit into
mainfrom
jay/kernel-telemetry-forwarding
Open

Forward telemetry config to kernel backend#451
jay-xiao446 wants to merge 1 commit into
mainfrom
jay/kernel-telemetry-forwarding

Conversation

@jay-xiao446

Copy link
Copy Markdown
Collaborator

Summary

  • disable Go wrapper telemetry for all kernel-backed connections
  • thread enableTelemetry, telemetry_batch_size, and driver/runtime identity into kernel config
  • update kernel telemetry docs and unit coverage

Tests

  • go test ./...
  • go test ./telemetry
  • go test -run 'TestBuildKernelConfig|TestKernelConfigFieldsClassified|TestKernelSkipsDriverTelemetry' .

Note: the current kernel C ABI does not yet expose telemetry setters, so this PR wires and tests the Go-side config shape for the future kernel handoff without adding a non-linkable cgo call.

@peco-review-bot peco-review-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Verdict: 1 Medium

Looks good overall — the telemetry-skip logic, kernel config forwarding, and unit coverage are coherent and the doc updates match the new behavior. One medium maintainability concern: removing the RecordConnectionConfig call leaves kernel_telemetry.go (and its helpers) as dead production code reachable only from its test.

Other findings

  • 🟡 Medium — This PR removes the sole production caller of kernelConnectionTelemetry — the conn.telemetry.RecordConnectionConfig(ctx, conn.id, kernelConnectionTelemetry(c.cfg)) block that previously lived in connector.go. After this change, kernelConnectionTelemetry (and its transitive helpers kernelUsesProxy / kernelAuthMech that are only reached through it) are referenced only from kernel_telemetry_test.go. That makes the whole kernel_telemetry.go file dead production code: it builds the DriverConnectionParameters telemetry payload that nothing emits anymore, and staticcheck's unused-code check won't flag it because the test still references it.

Since the kernel path now skips the Go telemetry interceptor entirely, this connection-config payload is never produced. Either delete kernel_telemetry.go (and its test), or, if it's being kept as scaffolding for the future kernel handoff, add a comment stating that and note it's intentionally test-only for now. As-is it reads as live code and will mislead the next reader into thinking kernel connection-config telemetry is still emitted.

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