Skip to content
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

Zipkin bad json encoding #316

Closed
n0tl3ss opened this issue Jun 7, 2023 · 8 comments
Closed

Zipkin bad json encoding #316

n0tl3ss opened this issue Jun 7, 2023 · 8 comments
Assignees

Comments

@n0tl3ss
Copy link
Member

n0tl3ss commented Jun 7, 2023

Expected Behavior

Spans to be valid json array

Actual Behaviour

Spans array is not properly json encoded

POST /api/v2/spans HTTP/1.1
host: localhost:9411
Connection: close
content-type: application/json
Content-Length: 284

][{"traceId":"2f1dd400d27b399a","id":"2f1dd400d27b399a","kind":"SERVER","name":"get","timestamp":1686134923454707,"duration":9948,"localEndpoint":{"serviceName":"tracing-test","ipv4":"192.168.205.1"},"tags":{"error":"404","http.method":"GET","http.path":"/","http.status_code":"404"}}

Steps To Reproduce

  1. Create application that will use zipking tracing (opentracing)
  2. Visit page that doesn't exists (returns 404)
  3. Observe zipkin client request body

Environment Information

No response

Example Application

No response

Version

3.9.3

@n0tl3ss n0tl3ss changed the title Zipkin json encoding wrong Zipkin bad json encoding Jun 7, 2023
@yawkat
Copy link
Member

yawkat commented Jun 7, 2023

does it still happen with 4.0?

@graemerocher
Copy link
Contributor

not sure, but a fix needs to be applied to 3.x as well as it impacts teams using 3.x

@yawkat
Copy link
Member

yawkat commented Jun 7, 2023

ok

@yawkat
Copy link
Member

yawkat commented Jun 7, 2023

how exactly do i reproduce this? i tried a simple test project and the json was valid:
image

@graemerocher
Copy link
Contributor

@n0tl3ss can you help @yawkat reproduce

@n0tl3ss
Copy link
Member Author

n0tl3ss commented Jun 7, 2023

@yawkat When I put it through proxy I can see that issue exists:
image
But without proxy spans are somehow correctly sent to zipkin service.

You can also put zipkin url from https://public.requestbin.com/ and you will get something like:

image

@yawkat
Copy link
Member

yawkat commented Jun 7, 2023

can you share your example project?

@n0tl3ss
Copy link
Member Author

n0tl3ss commented Jun 7, 2023

tracing-demo-json.zip

It is basic project with zipkin from launcher, just change configuration to:

micronaut:
  application:
    name: tracingDemoJson
netty:
  default:
    allocator:
      max-order: 3
tracing:
  zipkin:
    enabled: true
    http:
      url: <url-from-requestbin>
    sampler:
      probability: 1

yawkat added a commit to micronaut-projects/micronaut-core that referenced this issue Jun 8, 2023
HttpStreamsHandler delays writing the first HttpContent to when the request has been fully written. The second HttpContent was not delayed however, so when writing the request took time (apparently with TLS?), the second HttpContent was written before the first.

This patch delays all further writes (normal, complete and error) until the first content has been written.

Should fix micronaut-projects/micronaut-tracing#316
yawkat added a commit to micronaut-projects/micronaut-core that referenced this issue Jun 8, 2023
HttpStreamsHandler delays writing the first HttpContent to when the request has been fully written. The second HttpContent was not delayed however, so when writing the request took time (apparently with TLS?), the second HttpContent was written before the first.

This patch delays all further writes (normal, complete and error) until the first content has been written.

Should fix micronaut-projects/micronaut-tracing#316
graemerocher pushed a commit to micronaut-projects/micronaut-core that referenced this issue Jun 8, 2023
HttpStreamsHandler delays writing the first HttpContent to when the request has been fully written. The second HttpContent was not delayed however, so when writing the request took time (apparently with TLS?), the second HttpContent was written before the first.

This patch delays all further writes (normal, complete and error) until the first content has been written.

Should fix micronaut-projects/micronaut-tracing#316
@yawkat yawkat closed this as completed Aug 4, 2023
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

No branches or pull requests

3 participants