test(observability): verify startup with a newer host OTel SDK

[Kiran01bm]

Summary

Add a consumer-module startup test proving a host binary with a newer OpenTelemetry SDK can initialize schemabot telemetry.

Why

Host binaries embed this server as a Go module and resolve their own OTel SDK version, so a semconv schema URL conflict in telemetry init cannot reproduce inside this module — both SDK and semconv resolve together here. The existing unit test simulates a foreign base resource, but nothing exercised the real failure mode: a consumer whose resource.Default() carries a different schema URL than the semconv version this repo compiles against.

What

• e2e/consumermodule/: a nested Go module that pins go.opentelemetry.io/otel/sdk ahead of the parent repo and calls api.SetupTelemetry. A precondition assertion fails loudly if the pins ever drift into alignment (telling the maintainer to bump the consumer pin) so the test can't silently stop testing the conflict.
• make test-consumer-module, included in make test and run as a step in the Unit Tests CI job.

Verified: the test fails against the pre-fix telemetry code with conflicting Schema URL: 1.43.0 and 1.40.0 and passes on main.

[Copilot]

Pull request overview

Adds a “consumer module” test harness to validate SchemaBot’s telemetry startup compatibility when embedded by a host binary that resolves a newer OpenTelemetry SDK version (the real-world scenario that can trigger semconv schema URL merge conflicts).

Changes:

• Introduces a nested Go module under e2e/consumermodule/ that pins a newer go.opentelemetry.io/otel/sdk and exercises api.SetupTelemetry.
• Wires the new consumer-module test into make test and runs it as an explicit step in the Unit Tests GitHub Actions job.
• Adds the consumer module’s go.mod/go.sum to make the scenario deterministic and reproducible in CI.

Reviewed changes

Copilot reviewed 4 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
Makefile	Adds test-consumer-module and includes it in make test.
e2e/consumermodule/go.mod	Defines a nested consumer module that pins newer OTel SDK versions and replaces the parent module locally.
e2e/consumermodule/go.sum	Captures the nested module dependency resolution for repeatable CI runs.
e2e/consumermodule/consumer_test.go	Adds a startup test that calls api.SetupTelemetry under a newer host OTel SDK.
.github/workflows/test.yaml	Runs the consumer-module startup test in the Unit Tests job.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

[aparajon]

🤖 Adversarial correctness review, requested by Armand and performed by his agent. Reviewed at head fe6cc98.

Verdict: this is a genuinely load-bearing test and it's safe to land. I attacked the thing that matters most for a canary test — whether it can quietly stop testing anything — and it holds: I regressed telemetryResource to a schema-bearing override and the test failed with exactly conflicting Schema URL: 1.43.0 and 1.40.0, so it really does guard the schemaless merge rather than just asserting startup succeeds. The production delta is a single exported const. Nothing blocks; the three items below are maintenance hardening.

Findings

1. The mirrored replace directives can drift out of sync with the parent module, silently. e2e/consumermodule/go.mod hand-copies the parent's vitess.io/vitess and pingcap/tidb/pkg/parser replaces (correctly — replaces don't cross module boundaries, and the comment says so). But nothing ties them together: when the root module bumps either fork, this module keeps the stale pin. The near-term effect is invisible (this test only touches telemetry, so it passes either way, just against an older fork than the repo actually ships); the eventual effect is a compile failure in the consumer-module job whose cause sits in a file the author never edited. It fails loudly rather than silently green, which is why this isn't blocking — but the failure lands far from its cause.

2. The pre-commit hook never lints this module, so its files are only checked in CI. scripts/lint-fix.sh routes any staged path under e2e/ into lint_and_fix e2e ./e2e/..., and Go's package patterns don't descend into nested modules — I confirmed go list ./e2e/... from the root returns zero consumermodule packages. So staging e2e/consumermodule/consumer_test.go runs the linter over a package set that excludes it, silently: no error, no coverage. The new lint (consumer-module) CI job does catch it, so this is a local-loop gap rather than a hole in the gate, but AGENTS.md presents the hook as the commit contract.

3. (nit) The test isn't hermetic against OTel environment variables. SetupTelemetry branches on OTEL_EXPORTER_OTLP_ENDPOINT, so a developer who has it exported runs a different code path than the one under test — building real OTLP exporters, with Shutdown then attempting a flush. The service_name="schemabot" assertion itself is robust (the schemaless override wins the resource.Merge precedence, so OTEL_SERVICE_NAME can't displace it), but a canary test is worth pinning to one path.

Action items

1. (Finding 1) Add a guard that the mirrored replaces still match the parent's — a few lines in the consumer test reading both go.mod files and comparing the replace lines would fail with a clear "mirror the parent's replace directives" message, in the same spirit as the schema-URL precondition already in this test.
2. (Finding 2) In scripts/lint-fix.sh, branch staged paths matching ^e2e/consumermodule/ to a run with that directory as the working directory, so the hook covers the module the way the CI matrix now does.
3. (optional) t.Setenv("OTEL_EXPORTER_OTLP_ENDPOINT", "") at the top of the test to pin the Prometheus-only path regardless of the developer's environment.

Verified (tried to break, couldn't)

Ran the test at head and it passes; regressed telemetryResource back to resource.NewWithAttributes(semconv.SchemaURL, …) and confirmed the test fails with the exact conflict the PR body claims, then restored — so the test is proven to catch the regression it guards, not merely to pass. Confirmed the precondition can't silently degrade: equal schema URLs fail loudly with a bump instruction, and the empty-URL case is covered by the assertion added after the automated-reviewer thread; the guard also stays valid if the pins ever invert, since the schemaless merge is what's being tested in either direction. Confirmed module isolation — root go list ./... and ./e2e/... both exclude the nested module, and go build ./... is unaffected, so the root suite, vet, and build can't be perturbed by it. Confirmed the module actually resolves (the replace … => ../.. is present alongside the two mirrored fork replaces) and that make test-consumer-module runs it standalone. Confirmed the CI path filter is coarse enough (** minus docs) that a consumer-module-only change still triggers the Unit Tests job that runs it, and that the cache key now hashes both go.sum files with a prefix restore-keys fallback intact. Leak check passed on the body, diff, and this comment. All 34 checks pass.

This review was generated by Claude Code (claude-opus-5).

[aparajon]

🤖 Approving on Armand's behalf after the adversarial correctness review above (no blocking findings). This stamp was left by Claude Code (claude-opus-5).

[Kiran01bm]

🤖 Review response — created by Kiran's code review agent (Amp, Claude Opus 4.5) — pull/1015,

All three findings addressed:

1. Replace-directive drift — fixed: TestReplaceDirectivesMirrorParent parses both go.mod files (single-line and block forms) and requires the nested module's replaces to equal the parent's, excluding the ../.. self-replace. Mutation-tested with a stale-but-valid vitess pin: fails with the intended mismatch message.
2. Pre-commit hook skips the nested module — fixed: scripts/lint-fix.sh now routes staged e2e/consumermodule/ paths to a golangci-lint run from that directory (with --fix, re-staging, and --new-from-rev preserved), mirroring the CI consumer-module lint job.
3. OTel env hermeticity (nit) — fixed: t.Setenv("OTEL_EXPORTER_OTLP_ENDPOINT", "") pins the Prometheus-only path.