Skip to content

build(sketchcollector): wire asap-precompute-go replace directive - #231

Merged
zzylol merged 1 commit into
mainfrom
phase2/sketchcollector-asap-precompute-replace
May 4, 2026
Merged

zzylol merged 1 commit into
mainfrom
phase2/sketchcollector-asap-precompute-replace

Conversation

@zzylol

@zzylol zzylol commented May 4, 2026

Copy link
Copy Markdown
Contributor

Why this PR

The OTel collector distribution sketchcollector is assembled by OCB from opentelemetry-collector-contrib-patch/cmd/sketchcollector/builder-config.yaml. It bundles all five sketch processors (ddsketch, KLL, HLL, countsketch, countminsketch) plus their merge variants into a single binary.

After Phase 2 shim refactors landed in #226 - #230, every sketch processor's go.mod now requires the private module github.com/ProjectASAP/asap-precompute-go. That module does not exist on the public Go proxy / sum.golang.org, so the OCB-driven go mod tidy fails:

github.com/ProjectASAP/asap-precompute-go: ... replacement directory ... does not exist

This blocks the user's "single sketchcollector binary including all sketches plugins" workflow today.

What this PR does (two-file change)

  1. opentelemetry-collector-contrib-patch/cmd/sketchcollector/builder-config.yaml — add a replaces: entry that redirects github.com/ProjectASAP/asap-precompute-go to the in-repo checkout. Mirrors the existing sketchlib-go pattern. Path note: asap-precompute-go lives as a subdirectory of this repo (ASAPCollector/asap-precompute-go), unlike sketchlib-go which is a sibling repo, so the relative path is ../../../asap-precompute-go (one fewer .. than the sketchlib-go entry).

  2. build_sketchcollector.sh — defensively append the same replace to the OCB-generated go.mod if it isn't already present, mirroring the existing sketchlib-go grep+append guard. This handles the case where OCB strips replaces during go.mod regeneration.

The intent of one binary including all five sketch plugins (per builder-config.yaml lines 147-156) is preserved — this PR just makes the OCB-generated go.mod resolve asap-precompute-go to the local checkout instead of failing on a missing module-proxy lookup.

Verification

  • Ran ./build_sketchcollector.sh with these two file changes against current origin/main (which has all five Phase-2 shim PRs merged): build succeeds end-to-end. OCB tidy resolves the replace directive, compiles, and emits cmd/sketchcollector/sketchcollector (~470 MB binary).
  • Inspected the OCB-generated cmd/sketchcollector/go.mod after the build — both private-module replaces are wired correctly:
    replace github.com/ProjectASAP/sketchlib-go => ../../../../sketchlib-go
    replace github.com/ProjectASAP/asap-precompute-go => ../../../asap-precompute-go
    
  • Without the fix, the same script fails with replacement directory ... does not exist on asap-precompute-go during go mod tidy.

Test plan

  • ./build_sketchcollector.sh produces a working sketchcollector binary on origin/main with the five Phase-2 shim PRs merged.
  • No submodule pointer drift committed.
  • No structural refactor of the build script — only the two-line defensive grep+append guard added.

Out of scope

Independent of and parallel to PR phase2/snapshot-cache-correctness-and-common-wrappers.

Authoring note

This PR was authored by an automated agent (Claude Code).

View Claude Code session

Each Phase-2 shim depends on github.com/ProjectASAP/asap-precompute-go,
a sibling private module with no module-proxy presence. Mirror the
sketchlib-go replace pattern so OCB resolves it to the local checkout.

Without this, the multi-plugin sketchcollector binary fails to build
once any shim PR (#226-#230) merges.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@zzylol
zzylol merged commit 7840768 into main May 4, 2026
@zzylol
zzylol deleted the phase2/sketchcollector-asap-precompute-replace branch May 4, 2026 14:02
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