Backport Redis instrumentation to SB-1.x #193
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…b#177) * Make JDBC tracer configurable through Spring properties Add properties for "withActiveSpanOnly" and "ignoreStatement" so the behaviour of the JDBC tracer can be controlled through application.yaml * Formatting fixes Fix indentation and licences headers on new test classes. * Fix pom ordering * Rename package, fix tests * Add default test, document properties * Revert "Add default test, document properties" This reverts commit bdbf55a. * Add Default test * Add documentation for new properties * Revert "Add documentation for new properties" This reverts commit 5274ddb. * Revert "Add Default test" This reverts commit 1a72821. * Revert "Revert "Add documentation for new properties"" This reverts commit 47353f2. * Add identical test * Empty out test * Add empty test * Delete defaults test
* [issue-69] Redis instrumentation * [issue-69] PR changes * [issue-69] Fix dependecies
|
@pavolloffay workaround is to use |
|
@malafeev with 0.0.7 I get the same error. The 0.0.7 uses SB 2 not 1.x https://github.com/opentracing-contrib/java-redis-client/blob/0.0.7/opentracing-redis-spring/pom.xml#L30 |
Signed-off-by: Pavol Loffay <ploffay@redhat.com>
|
Looks like solution is only to add support of sping-boot 1.5x to opentracing-redis-spring. |
Or create a submodule in master branch - as you prefer. |
|
I'm not sure what's the best. |
|
Well the plan is to support it. For me it's easier to support it as module than go to a separate branch |
|
@malafeev ping me when the artifact is out please. |
Signed-off-by: Pavol Loffay <ploffay@redhat.com>
pavolloffay
commented
Jan 23, 2019
| redisTemplate.opsForValue().set(100L, "Some value here"); | ||
| assertEquals(1, tracer.finishedSpans().size()); | ||
| // TODO it's null here, but in SB 2.x it works | ||
| // assertEquals("SET", tracer.finishedSpans().get(0).tags().get("command")); |
There was a problem hiding this comment.
@malafeev why the command is not set when using opentracing-redis-spring-data? I get this regression when compared to SB 2
There was a problem hiding this comment.
command is an operationName now, do we need to duplicate tag?
currentTracer.buildSpan(command)
There was a problem hiding this comment.
no need for duplication. Has it also changed in the instrumentation for SB 2.x?
Signed-off-by: Pavol Loffay <ploffay@redhat.com>
pavolloffay
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Resolves #69
At the moment it's blocked by opentracing-contrib/java-redis-client#28. The redis instrumentation fails on SB 1.x.
@malafeev any workarounds are appreciated.