feat(tags): http #827
feat(tags): http #827
Conversation
FrankSpitulski
requested review from
anuraaga,
iNikem,
jkwatson,
trask and
tylerbenson
as code owners
|
fixing tests, but I figured I'd put it up here for people to review |
...rc/main/java/io/opentelemetry/auto/bootstrap/instrumentation/decorator/HttpServerTracer.java
Show resolved
Hide resolved
...rc/main/java/io/opentelemetry/auto/bootstrap/instrumentation/decorator/HttpServerTracer.java
Outdated
Show resolved
Hide resolved
agent-bootstrap/src/main/java/io/opentelemetry/auto/typedspan/DelegatingSpan.java
Outdated
Show resolved
Hide resolved
...main/java/io/opentelemetry/auto/instrumentation/netty/v3_8/server/NettyHttpServerTracer.java
Outdated
Show resolved
Hide resolved
...rc/main/java/io/opentelemetry/auto/bootstrap/instrumentation/decorator/HttpServerTracer.java
Outdated
Show resolved
Hide resolved
FrankSpitulski
force-pushed
the
feat/tags/http
branch
2 times, most recently
from
44e5261
to
f4a7fc9
Compare
...rc/main/java/io/opentelemetry/auto/bootstrap/instrumentation/decorator/HttpServerTracer.java
Show resolved
Hide resolved
...rc/main/java/io/opentelemetry/auto/bootstrap/instrumentation/decorator/HttpServerTracer.java
Outdated
Show resolved
Hide resolved
|
@trask I think this may be a sporadic test failure, works on my machine and it's a timeout error 🤷 |
flavour, user agent, client ip
FrankSpitulski
force-pushed
the
feat/tags/http
branch
from
7708ca8
to
3eddbad
Compare
| return peerHostIP(connection); | ||
| } | ||
|
|
||
| private static String extractForwardedFor(String forwarded) { |
There was a problem hiding this comment.
Can you add unit test for this method (to mitigate against the extra complexity we asked for 😄)?
There was a problem hiding this comment.
added tests, caught an edge case
There was a problem hiding this comment.
getting another issue with latestDepTest though.
./gradlew :instrumentation:grpc-1.5:latestDepTest fails
./gradlew :instrumentation:grpc-1.5:test works
...rc/main/java/io/opentelemetry/auto/bootstrap/instrumentation/decorator/HttpServerTracer.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Thanks @FrankSpitulski! I'll look into the latestDepTest issue and let you know once that's resolved in master.
|
I forgot that I can merge master into your branch 😄 |
flavour, user agent, client ip
I had to make a new type of tracer method that takes both the request and the connection because netty has the flavour on the request while armeria has the flavour on the connection. Also, I needed a fallback for the client ip which relies on the connection.
Closes #776