-
Notifications
You must be signed in to change notification settings - Fork 619
feat(redis): consolidate redis v2,3 and redis v4 instrumentation to one package #2878
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
f44b587
e776bc6
2329836
65b09d0
ca617a5
4ecf3a8
537cd34
de9f42e
b99ef55
5279606
63638da
1ec39b4
5b2c10b
3945aab
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,11 @@ | ||
| redis: | ||
| versions: | ||
| include: '>=2.6.0 <4' | ||
| mode: latest-minors | ||
| commands: npm run test | ||
| - versions: | ||
| include: '>=2.6.0 <4' | ||
| mode: latest-minors | ||
| commands: npm run test-v2-v3-run | ||
| - versions: | ||
| include: '>=4 <5' | ||
| # "4.6.9" was a bad release that accidentally broke node v14 support. | ||
| exclude: "4.6.9" | ||
| mode: latest-minors | ||
| commands: npm run test-v4-run |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,12 +1,16 @@ | ||
| { | ||
| "name": "@opentelemetry/instrumentation-redis", | ||
| "version": "0.49.0", | ||
| "description": "OpenTelemetry instrumentation for `redis` v2 and v3 database client for Redis", | ||
| "description": "OpenTelemetry instrumentation for `redis` database client for Redis", | ||
| "main": "build/src/index.js", | ||
| "types": "build/src/index.d.ts", | ||
| "repository": "open-telemetry/opentelemetry-js-contrib", | ||
| "scripts": { | ||
| "test": "nyc mocha 'test/**/*.test.ts'", | ||
| "test": "npm run test-v2-v3 && npm run test-v4", | ||
| "test-v2-v3": "tav redis 3.1.2 npm run test-v2-v3-run", | ||
| "test-v4": "tav redis 4.7.1 npm run test-v4-run", | ||
| "test-v2-v3-run": "nyc mocha --no-clean --require '@opentelemetry/contrib-test-utils' 'test/v2-v3/*.test.ts'", | ||
| "test-v4-run": "nyc mocha --no-clean --require '@opentelemetry/contrib-test-utils' 'test/v4/*.test.ts'", | ||
|
Comment on lines
+9
to
+13
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. reviewer note: this is aligned with
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. IMO we should be leaving testing of multiple versions of the target library to I have an alternative PR that builds on yours that shows what I mean: #2915 |
||
| "test:debug": "cross-env RUN_REDIS_TESTS_LOCAL=true mocha --inspect-brk --no-timeouts 'test/**/*.test.ts'", | ||
| "test:local": "cross-env RUN_REDIS_TESTS_LOCAL=true npm run test", | ||
| "test:docker:run": "docker run --rm -d --name otel-redis -p 63790:6379 redis:alpine", | ||
|
|
@@ -59,6 +63,7 @@ | |
| "cross-env": "7.0.3", | ||
| "nyc": "17.1.0", | ||
| "redis": "3.1.2", | ||
| "redis-v4": "npm:[email protected]", | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. reviewer note: this is needed for the When running the tests, |
||
| "rimraf": "5.0.10", | ||
| "test-all-versions": "6.1.0", | ||
| "typescript": "5.0.4" | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.