Conversation
Signed-off-by: Valery Piashchynski <piashchynski.valery@gmail.com>
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (4)
📒 Files selected for processing (16)
✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR migrates the BoltDB RoadRunner plugin and its test harness to the v6 module/API ecosystem, including updated Jobs/KV plugin interfaces and refreshed CI/test instrumentation.
Changes:
- Bump module path to
github.com/roadrunner-server/boltdb/v6and migrate toapi-plugins/v6(Jobs/KV) and newer Goridge/OTel deps. - Refactor OTEL-related tests to use an in-memory OpenTelemetry exporter instead of querying an external Zipkin endpoint.
- Update CI workflow to generate/upload coverage artifacts under the new module path and simplify coverage flags.
Reviewed changes
Copilot reviewed 15 out of 20 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/pkgs.txt | Removes the package list previously used for -coverpkg. |
| tests/php_test_files/composer.json | Adjusts PHP test dependencies (HTTP client stack). |
| tests/mock/tracer.go | Adds an in-memory tracer provider/exporter for tests. |
| tests/helpers/helpers.go | Updates API imports + RPC dial behavior (DialContext) and modernizes loops. |
| tests/go.mod | Updates test module dependencies to v6 APIs and newer OTel versions; updates replace to v6. |
| tests/go.sum | Syncs sums with updated test dependencies. |
| tests/boltdb_test.go | Switches OTEL test from Zipkin HTTP polling to in-memory span assertions; updates imports to v6. |
| plugin.go | Migrates to api-plugins/v6, adds tracer collection via Endure, and updates constructor signatures to accept context.Context. |
| go.mod | Changes module path to /v6, updates deps, and adds exclude entries for viper. |
| go.sum | Syncs sums with updated root dependencies. |
| boltkv/driver.go | Migrates KV storage interface to api-plugins/v6 and adds context-aware method signatures and span creation. |
| boltjobs/listener.go | Cleans up listener code/comments and simplifies UTC handling. |
| boltjobs/item.go | Switches JSON implementation to stdlib and tightens error handling in requeue path. |
| boltjobs/driver.go | Migrates Jobs driver to api-plugins/v6, adds ctx params to constructors, and improves some internals/logging. |
| .golangci.yml | Removes dupl and drops test-specific linter rule overrides. |
| .gitignore | Adds **/composer.lock to ignores. |
| .github/workflows/linux.yml | Updates CI matrix/coverage collection, artifact handling, and codecov preparation. |
| .github/dependabot.yml | Removes explicit reviewers configuration. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| func (d *Driver) Has(ctx context.Context, keys ...string) (map[string]bool, error) { | ||
| const op = errors.Op("boltdb_driver_has") | ||
| _, span := d.tracer.Tracer(tracerName).Start(context.Background(), "boltdb:has") | ||
| _, span := d.tracer.Tracer(tracerName).Start(ctx, "boltdb:has") | ||
| defer span.End() | ||
|
|
There was a problem hiding this comment.
d.tracer can be nil (e.g., when the boltdb plugin is registered without any Tracer provider), but these methods unconditionally call d.tracer.Tracer(...), which will panic at runtime. Add a nil-safe fallback (e.g., initialize a default sdktrace.NewTracerProvider() in NewBoltDBDriver when tracer == nil, or guard before starting spans).
| func (p *Plugin) Init(log Logger, cfg Configurer) error { | ||
| p.log = log.NamedLogger(PluginName) | ||
| p.log = log.NamedLogger(pluginName) | ||
| p.cfg = cfg |
There was a problem hiding this comment.
The plugin no longer initializes a default tracer provider. If nothing in the container matches the Tracer interface, p.tracer remains nil and is passed into KV/JOBS constructors (KV currently panics when starting spans). Consider setting p.tracer to a default provider in Init (and/or returning a clear error if no tracer was collected) to avoid runtime crashes in configurations that don’t include an OTEL/tracer plugin.
| p.cfg = cfg | |
| p.cfg = cfg | |
| // Initialize a default (noop) tracer provider so p.tracer is never nil. | |
| // This can be overridden later via the Tracer dependency collected in Collects(). | |
| p.tracer = sdktrace.NewTracerProvider() |
| - run: | | ||
| echo 'mode: atomic' > summary.txt | ||
| tail -q -n +2 *.out >> summary.txt | ||
| sed -i '2,${/roadrunner/!d}' summary.txt | ||
|
|
||
| tail -q -n +2 coverage/*.out >> summary.txt | ||
| awk ' | ||
| NR == 1 { print; next } | ||
| /^github\.com\/roadrunner-server\/boltdb\/v6\// { | ||
| sub(/^github\.com\/roadrunner-server\/boltdb\/v6\//, "", $0) | ||
| } | ||
| ' summary.txt > summary.filtered.txt | ||
| mv summary.filtered.txt summary.txt |
There was a problem hiding this comment.
The awk filter keeps only coverage lines starting with github.com/roadrunner-server/boltdb/v6/, but both test commands run go test with explicit file lists (e.g. go test ... plugin.go plugin_test.go / ... boltdb_test.go), which typically produces coverprofile entries prefixed with command-line-arguments/.... This filtering will likely drop all coverage data and upload an effectively empty report. Adjust the test invocation to use package paths (e.g. go test ./... or go test . / go test ./tests) or broaden the filter to include the actual coverprofile path prefix.
Reason for This PR
Description of Changes
License Acceptance
By submitting this pull request, I confirm that my contribution is made under
the terms of the MIT license.
PR Checklist
[Author TODO: Meet these criteria.][Reviewer TODO: Verify that these criteria are met. Request changes if not]git commit -s).CHANGELOG.md.