zipkin
zipkin copied to clipboard
openzipkin
fix NullPointerException when parser.currentName() is null
when parser.currentName() is null, switch will throw NullPointerException
ANy news on this?
Sorry with a late response, I find it happened when zipkin client is in an old version which has json array in span data, like below:
{
"timestamp_millis": 1648189127998,
"_q":
[
"http.method",
"http.method=GET",
"http.path",
"http.path=/getId",
"mvc.controller.class",
"mvc.controller.class=IdCenterController",
"mvc.controller.method",
"mvc.controller.method=getId"
],
"traceId": "9cc28a3ad42757af",
"parentId": "9cc28a3ad42757af",
"id": "6ecb2ed650bb52c8",
"kind": "SERVER",
"name": "get /getid",
"timestamp": 1648189127998059,
"duration": 717,
"localEndpoint":
{
"serviceName": "id-center-service",
"ipv4": "*.*8.*4.*0"
},
"remoteEndpoint":
{
"ipv4": "*.*8.*4.*0"
},
"tags":
{
"http.method": "GET",
"http.path": "/getId",
"mvc.controller.class": "IdCenterController",
"mvc.controller.method": "getId"
},
"shared": true
}
when _q gets parsed, NullPointerException happened.
Shall we close this then?
I think this pr can fix NullPointerExceptin when zipkin client in old version but zipkin server in new version, and zipkin client is integrated in business project which means it‘s not an easy thing to update, but zipkin server is an independent service which means it's easy to update.
I hope you can accept this pr.
Hello. Is this expected to be merged soon because I am running into the same issue. I converted a string to an array object using the elasticsearch ingest node processors and when it tries parsing it on the zipkin server side it fails.
I'm trying to build a docker image local from the fix code and the build seems to be failing on the zipkin-lens project. Something to do with linting. See error below.
[install 5/5] RUN /code/build-bin/maven/maven_build_or_unjar io.zipkin zipkin-server master exec && mv zipkin-server zipkin && /code/build-bin/maven/maven_build_or_unjar io.zipkin zipkin-server master slim && mv zipkin-server zipkin-slim:
#17 0.436 *** Building snapshot from source...
#17 73.97 [ERROR] Failed to execute goal com.github.eirslett:frontend-maven-plugin:1.11.0:npm (npm lint) on project zipkin-lens: Failed to run task: 'npm run lint' failed. org.apache.commons.exec.ExecuteException: Process exited with an error: 254 (Exit value: 254) -> [Help 1]
So I was able to fix the issue mentioned but it required me to update the dockerfile under buiild-bin/docker and change the java version and the artifact version in order to get it working. The image that it built though didn't have the changes. In the docs it says that its possible to build without the CI but I can't figure out how to get the image to compile and package the code into a final docker image that I can tag to my own repo. @jcchavezs Were you able to get the full project built as a final docker image?
Hello, Since it doesn't look like this change will be going out soon can someone please tell me how the zipkin server docker image is built? I want to build my own version of this image locally and push to my own registry. I was trying to use the build-bin/docker/docker_build but the image created did not have changes from the code in the repo. Am I missing something?
We never said this would not be merged. I was just asking for a unit test. Do you mind adding it so I can merge?
@eirslett any idea on this failure? https://github.com/openzipkin/zipkin/actions/runs/4266238369/jobs/7651835336#step:7:9
Yes I can add unit tests for it. The failure that is mentioned looks like a networking error that might have happened when the build was running. That is just my observation from the output.
@jcchavezs I created a pull request here as I did not want to open up another pull request. Please merge it into this pull request if you can. I added two tests and made a change to support when an array is provided as a value to the tags. Please let me know if this is good enough or if you need some more coverage.
@weishaofei There are two pull requests on the code you forked with your change. Please use the second pull request that I made as it has the tests along with an extra fix I made for the tags.
Thanks for the test. So according to the cases what I understand is that we want to fail gracefully when an unknown type arrives and it seems to me this happens when a key in ES comes nil? is that correct? If so, can we only address that case instead accepting any sort of type data?
@jcchavezs Well sort of. There are two cases.
- The first case is if some provides extra data to the json in elastic and the name is null because it is not one of the names that are expected. That is the reason why it is null.
- The second case is when an array is provided as a value for tags. It tries to map the key to null and each of the values in the array to a null key. Here is the output of the second scenario:
Parser current name nums
Parser value null
Parser current name null
Parser value 1
Parser current name null
Parser value 2
Parser current name null
Parser value 3
Parser current name nums
Parser value null
As you can see both key and value in this case are null but not at the same time.
Hello, Can someone help with this issue. The zipkin build fails when trying to build the zipkin lens project.
zipkin/zipkin-lens/src/components/DependenciesPage/DependenciesGraph.tsx(163,17):
Generic type 'ValueType' requires 2 type argument(s). TS2314
161 | const handleFilterChange = useCallback(
162 | (
> 163 | selected: ValueType<{ value: string }>,
| ^
164 | actionMeta: ActionMeta<{ value: string; label: string }>,
165 | ) => {
166 | setFocusedNodeName('');
npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! [email protected] build: `react-scripts build`
npm ERR! Exit status 1
npm ERR!
npm ERR! Failed at the [email protected] build script.
There was no changes to the zipkin lens project at all.
I'm using node 13.14.0 which I am assuming is the right version based on the included nodes module.
Ping @tacigar
On Tue, 14 Mar 2023, 17:30 o2themar, @.***> wrote:
Hello, Can someone help with this issue. The zipkin build fails when trying to build the zipkin lens project.
zipkin/zipkin-lens/src/components/DependenciesPage/DependenciesGraph.tsx(163,17): Generic type 'ValueType' requires 2 type argument(s). TS2314
161 | const handleFilterChange = useCallback( 162 | (163 | selected: ValueType<{ value: string }>, | ^ 164 | actionMeta: ActionMeta<{ value: string; label: string }>, 165 | ) => { 166 | setFocusedNodeName('');
npm ERR! code ELIFECYCLE npm ERR! errno 1 npm ERR! @.*** build:
react-scripts buildnpm ERR! Exit status 1 npm ERR! npm ERR! Failed at the @.*** build script.There was no changes to the zipkin lens project at all.
— Reply to this email directly, view it on GitHub https://github.com/openzipkin/zipkin/pull/3437#issuecomment-1468432463, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAXOYAXQILULVCSNLMJ2ZRTW4CMKPANCNFSM5RTOWT4A . You are receiving this because you were mentioned.Message ID: @.***>
@tacigar Any thoughts on the issue? Also I am noticing when trying to run zipkin server in a docker container I keep getting request timeouts and the UI doesn't load any data. Could it be because its trying to collect the data and if the data is older than the current filtered time range it times out retrieving the data? I know there is an open issue about zipkin to elastic https://github.com/openzipkin/zipkin/issues/3433
@jcchavezs I was wondering if we will be moving forward with these changes? Is a release planned soon?
Hello. Just checking again to see if there is any progress on this. Haven't seen any changes in a while. Also noticed the master branch is way ahed of this branch but there is no conflict between the two branches so there shouldn't be any issues merging. However, when I merged the master to branch to my branch I did notice the mysql tests failing and then there was restriction on the Java version has to be between 1.8 >= and < 16. I couldn't build using the latest java version that was provided by openzipkin which is Java 17.
Hello @jcchavezs . Sorry I've been away for a while but I started to look at this again. I tried rebasing with my current fork and it didn't work. I then cloned the main master branch from openzipkin and reran the tests for mysql and it also doesn't work so the problem even exists on the main master. Unless there is something wrong with my local setup in terms of java version it seems the build is failing no matter what branch.
Update from my side. I was testing more of the functionality and realized that ignoring the array value in the tags is not what should happen. Instead I changed it so that it parses the array so that it can be viewable in the tags. Now I can see the array value in the tags but the value is not searchable. For example, if I have tags: { ids: [1, 2, 3]} I won't be able to search for 1, 2, or 3 by providing the query _q: id=1 or _q: id=2 or _q: id=3. None of these would return results with the fix I made. So I looked deeper and though to change the Map value type for the annotationQuery to type Object. This is so it can have a type array returned when calling the method parseAnnotationQuery in the class QueryRequest. Changing the map value type required me to change it in several files to get it to compile. But after all that it is still not searchable even though I provide the value as an array when returning from parseAnnotationQuery. Any ideas on what else I need to change to get this working so I can search multiple value fields for any tag?
@codefromthecrypt please fix conflicts and merge this pr, or add a parser of the _q field?
This change is in the same line as this https://github.com/openzipkin/zipkin/pull/3503#issuecomment-1896131370 where basically something is writing data that is not in zipkin's format into zipkin's schema. We can change the NPE to a better message, but for the same reason as the other, we will not support custom types in zipkin's schema.
At the moment there is not enough help on this project. Until that changes, we will not entertain any change that makes it easier to use unsupported data in our schema.
An acceptable change to this would be to fail with a better message, similar to how collector endpoint does. Feel free to raise a PR like that (to make the ES parser similar to our other one in how it fails).
From https://github.com/openzipkin/zipkin/pull/3693
@Test void niceErrorOnNonStringTagValue() {
String json = """
{
"traceId": "6b221d5bc9e6496c",
"name": "get-traces",
"id": "6b221d5bc9e6496c",
"tags": {
"error": true
}
}
""";
assertThatThrownBy(() -> SpanBytesDecoder.JSON_V2.decodeOne(json.getBytes(UTF_8)))
.hasMessageContaining("Expected a string but was BOOLEAN");
}
or from https://github.com/openzipkin/zipkin/pull/3695
/** Tags in zipkin v2 model are stringly typed. */
@Test void acceptSpans_decodingError_nonStringValue() {
byte[] bytes = """
{
"traceId": "6b221d5bc9e6496c",
"name": "get-traces",
"id": "6b221d5bc9e6496c",
"tags": {
"error": true
}
}
""".getBytes(UTF_8); // error tag has a bool instead of string value
collector.acceptSpans(bytes, SpanBytesDecoder.JSON_V2, callback);
verify(callback).onError(any(IllegalArgumentException.class));
assertDebugLogIs("Malformed reading List<Span> from json");
verify(metrics).incrementMessagesDropped();
}