Skip to content

Standardize obs with ex repo conventions + bug fixes + test coverage#5

Closed
clairevnext wants to merge 12 commits intomainfrom
standardize-with-ex-repo
Closed

Standardize obs with ex repo conventions + bug fixes + test coverage#5
clairevnext wants to merge 12 commits intomainfrom
standardize-with-ex-repo

Conversation

@clairevnext
Copy link
Copy Markdown
Contributor

Summary

Brings obs up to the standards used by the ex repo (the pattern the rest of our bold-minds Go libs follow), fixes several real bugs found along the way, and expands test coverage significantly. 11 commits of substantive work that has been sitting on a feature branch — time to land it.

Commits

Standardization

  • `b3a7d53` — feat: standardize repository with ex configuration
  • `c65f4dc` — chore: align config files with bold-minds org standards

Bug fixes

  • `2554453` — fix: correct tracer name prefix from 'traverzer' to 'obs' (wrong identifier was leaking into traces)
  • `b38f9c0` — fix: eliminate race condition in sampling manager
  • `1a8e89f` — fix: return proper no-op span when sampling rejects operation
  • `206ebfa` — fix: handle empty high-volume endpoints env var (previously panicked on empty config)

Test expansion

  • `69abb63` — test: add TrackedOperation lifecycle tests
  • `0ce069f` — test: add comprehensive business event system tests
  • `d59e074` — test: expand root package tests for client lifecycle and global functions

Docs + final polish

  • `d41bcdc` — docs: replace copy-pasted ex docs with obs-specific versions (docs were lifted from ex and still had ex-isms)
  • `a895896` — fix: address all code review findings (CRITICAL through LOW)

Verification

  • `go build ./...` clean
  • `go test -race ./...` — all 6 test packages pass, race detector clean

Test plan

  • CI `test.yaml` passes
  • Manual scan of diff against `ex` repo patterns to confirm config consistency

🤖 Generated with Claude Code

clairevnext and others added 12 commits August 10, 2025 12:54
- Add standard OSS files (CODE_OF_CONDUCT, CONTRIBUTING, LICENSE, SECURITY)
- Add GitHub configuration (.github/CODEOWNERS, dependabot.yml, templates)
- Add standardized test workflow with GitHub App badge automation
- Add golangci-lint configuration and gitignore
- Setup badges directory with placeholder badges
- Configure repository for professional OSS development

This standardization aligns the repository with bold-minds/ex for consistency
across the organization and enables automated badge generation with green status.
Replace math/rand with math/rand/v2 (thread-safe global since Go 1.22).
Remove the rng field and per-instance seeding; make shouldSampleAtRate a
package-level function. Add resetGlobal() for test isolation and 7
comprehensive tests covering concurrency, routing, boundaries, nil manager,
and runtime config updates.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
StartSpanWithSampling was returning oteltrace.SpanFromContext(ctx) for
unsampled operations, which is the parent span. Any attributes set on
the returned span would accumulate on the parent. Replace with
noop.NewTracerProvider().Tracer("").Start(...) so the returned span is
a genuine no-op that neither records nor propagates attributes.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
CRITICAL:
- Race on globalClient: protect with sync.RWMutex
- Race on Client.config: add sync.RWMutex to Client struct
- sampling.Init sync.Once silently drops second call: replace with
  SetManager that always creates a new Manager
- Nested timeout in Shutdown: remove inner timeout, honor caller's ctx
- Exporter leaked on resource.New failure: shut down exporter on error

HIGH:
- HighVolumeSampleRate validated even when no endpoints configured:
  make validation conditional on len(HighVolumeEndpoints) > 0
- Redundant string "duration" attribute: remove, keep only duration_ms
- Hardcoded db.system="neo4j": make dbSystem a parameter on
  AddDatabaseContext
- Raw Cypher in span attributes: rename to "statement", add sanitization
  warning to doc comments
- FinishSpanWithResult calls End() without documenting it: add doc

MEDIUM:
- resetGlobal in production file: move to sampling/export_test.go
- Outdated OTel v1.21.0 with transitive CVEs: upgrade to latest
- Config test assumes HighVolumeSampleRate always validated: fix test

LOW:
- Dead code: remove SimpleException/NewException from internal package
- Dead code: remove unused trackBatchEvents from examples/generic
- Dead config: remove testify from depguard allowlist
- Shutdown now nils h.tp to reflect stopped state

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@clairevnext
Copy link
Copy Markdown
Contributor Author

Redundant — all feature work was already merged via PR #1 (squash). Only net diff was a 2-line GOTOOLCHAIN removal, not pursuing separately. Closing + deleting branch.

@clairevnext clairevnext deleted the standardize-with-ex-repo branch April 13, 2026 00:27
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