net.sock.host.port and net.host.port may be mixed up (at least in Tomcat?)

[trask]

Looking at one of the smoke test outputs, it looks like net.sock.host.port and net.host.port may be mixed up:

App - STDERR: [otel.javaagent 2023-09-20 19:36:16:984 +0000] [http-nio-8080-exec-1] INFO io.opentelemetry.exporter.logging.LoggingSpanExporter - 'GET /greeting' : 279c6319ff60f05fb918f58f21df5e39 32c0419c90ed3134 SERVER [tracer: io.opentelemetry.tomcat-7.0:1.31.0-alpha-SNAPSHOT] AttributesMap{data={net.protocol.name=http, thread.id=33, net.protocol.version=1.1, http.scheme=http, net.sock.host.port=8080, http.method=GET, net.sock.peer.port=52638, http.status_code=200, http.route=/greeting, net.host.port=32780, http.response_content_length=3, net.sock.host.addr=172.18.0.3, thread.name=http-nio-8080-exec-1, user_agent.original=armeria/unknown, net.host.name=localhost, http.target=/greeting, net.sock.peer.addr=172.18.0.1}, capacity=128, totalAddedValues=17}

[mateuszrzeszutek]

Jetty sometimes gets it right:

'GET /app/headers' : 67425fd042f594e780d590c4eaff22a9 15c67955c10c6e14 SERVER [tracer: io.opentelemetry.jetty-8.0:1.31.0-alpha-SNAPSHOT] AttributesMap{data={thread.name=qtp-275443369-42, user_agent.original=Java/1.8.0_372, net.host.name=localhost, http.target=/app/headers, net.sock.peer.addr=127.0.0.1, http.status_code=200, net.protocol.name=http, http.scheme=http, net.protocol.version=1.1, net.host.port=8080, http.method=GET, net.sock.peer.port=39712, http.route=/app/headers, net.sock.host.addr=127.0.0.1, thread.id=42}, capacity=128, totalAddedValues=15}

And sometimes not:

'GET /app/jsp' : b5a995a1c1f709d7b4d657b993afd605 7fd576a28ea01dea SERVER [tracer: io.opentelemetry.jetty-8.0:1.31.0-alpha-SNAPSHOT] AttributesMap{data={thread.name=qtp-275443369-36, user_agent.original=armeria/unknown, net.host.name=localhost, http.target=/app/jsp, net.sock.peer.addr=172.18.0.1, http.status_code=200, net.protocol.name=http, http.scheme=http, net.protocol.version=1.1, net.host.port=32797, http.method=GET, net.sock.peer.port=44268, http.route=/app/jsp, net.sock.host.addr=172.18.0.3, net.sock.host.port=8080, thread.id=36}, capacity=128, totalAddedValues=16}

Liberty instrumentation gets it wrong too:

'GET /app/asyncgreeting' : 3f52d2fe4e5d695610f635eb3d444f82 5764698c76a3f6b4 SERVER [tracer: io.opentelemetry.liberty-20.0:1.31.0-alpha-SNAPSHOT] AttributesMap{data={thread.name=Default Executor-thread-1, net.sock.peer.port=43038, thread.id=42, net.sock.host.port=8080, user_agent.original=armeria/unknown, net.host.name=localhost, http.status_code=200, http.target=/app/asyncgreeting, net.sock.peer.addr=172.18.0.1, net.host.port=32796, net.protocol.name=http, http.method=GET, http.scheme=http, net.protocol.version=1.1, net.sock.host.addr=172.18.0.3, http.route=/app/asyncgreeting}, capacity=128, totalAddedValues=16}

But the servlet instrumentation on the same web server is correct (it does not report the sock port at all though):

'GET /app/headers' : 3f52d2fe4e5d695610f635eb3d444f82 fb75c53af81704a9 SERVER [tracer: io.opentelemetry.servlet-3.0:1.31.0-alpha-SNAPSHOT] AttributesMap{data={thread.name=Default Executor-thread-2, net.sock.peer.port=51758, thread.id=43, user_agent.original=Java/11.0.20, net.host.name=localhost, http.status_code=200, http.target=/app/headers, net.sock.peer.addr=127.0.0.1, net.host.port=8080, net.protocol.name=http, http.method=GET, http.scheme=http, net.protocol.version=1.1, net.sock.host.addr=127.0.0.1, http.route=/app/headers}, capacity=128, totalAddedValues=15}

Grizzly being right and wrong:

'GET /app/headers' : 25d7b2acd929fdd339614b509fef2237 2bf74d7ae74cbef4 SERVER [tracer: io.opentelemetry.grizzly-2.3:1.31.0-alpha-SNAPSHOT] AttributesMap{data={http.scheme=http, net.sock.peer.port=49732, net.protocol.version=1.1, thread.id=93, http.method=GET, net.protocol.name=http, http.route=/app/headers, net.sock.host.addr=127.0.0.1, net.host.port=8080, http.status_code=200, thread.name=http-thread-pool::http-listener-1(2), http.target=/app/headers, net.sock.peer.addr=127.0.0.1, user_agent.original=Java/17.0.7, net.host.name=localhost, net.transport=ip_tcp}, capacity=128, totalAddedValues=16}|#]

'GET /app/asyncgreeting' : 25d7b2acd929fdd339614b509fef2237 c26ef704100062c3 SERVER [tracer: io.opentelemetry.grizzly-2.3:1.31.0-alpha-SNAPSHOT] AttributesMap{data={http.scheme=http, net.sock.peer.port=46988, net.protocol.version=1.1, thread.id=92, http.method=GET, net.sock.host.port=8080, net.protocol.name=http, http.route=/app/asyncgreeting, net.sock.host.addr=172.18.0.3, net.host.port=32781, http.status_code=200, thread.name=http-thread-pool::http-listener-1(1), http.target=/app/asyncgreeting, net.sock.peer.addr=172.18.0.1, user_agent.original=armeria/unknown, net.host.name=localhost, net.transport=ip_tcp}, capacity=128, totalAddedValues=17}|#]

[mateuszrzeszutek]

I read the whole extractor/getter code again, ran some extra unit tests both for the extractors and some of the HTTP servers, and I haven't been able to reproduce that locally. I suspect that maybe reusing the request object by the server might introduce this bug, but then again I ran several HTTP servers tests with @TestInstance(PER_CLASS) (to force the server to reuse requests) but that came up with nothing.

[laurit]

Perhaps this is because 'GET /app/headers happens inside the application, GreetingServlet makes the request to 8080. The other requests like GET /app/asyncgreeting are made from outside of the container to the port that testcontainers maps to 8080.

[vasireddy99]

Does OTel Java agent version-2.0 still emitting these metrics ?

[laurit]

Does OTel Java agent version-2.0 still emitting these metrics ?

I think it does but these are now named network.peer.port and server.port

[trask]

@vasireddy99 check out https://github.com/open-telemetry/opentelemetry-java-instrumentation/releases/tag/v2.0.0 and https://github.com/open-telemetry/semantic-conventions/blob/main/docs/http/migration-guide.md#summary-of-changes

[vasireddy99]

@vasireddy99 check out https://github.com/open-telemetry/opentelemetry-java-instrumentation/releases/tag/v2.0.0 and https://github.com/open-telemetry/semantic-conventions/blob/main/docs/http/migration-guide.md#summary-of-changes

Yes yes, I saw that and commenting if this is issue still valid and need to be modified.

[laurit]

As described in #9523 (comment) I suspect that this is not an issue at all but rather these attributes get different values in our smoke test suite depending on where the request originates from.

[vasireddy99]

@vasireddy99 check out https://github.com/open-telemetry/opentelemetry-java-instrumentation/releases/tag/v2.0.0 and https://github.com/open-telemetry/semantic-conventions/blob/main/docs/http/migration-guide.md#summary-of-changes

Hi @trask It seems, no breaking changes are mentioned specifically for net.sock.host.port attribute in either of those.

[vasireddy99]

Does OTel Java agent version-2.0 still emitting these metrics ?

I think it does but these are now named network.peer.port and server.port

It is renamed for the attribute net.sock.peer.addr → network.peer.address. net.sock.host.port is not defined and i think i am missing those metrics in v2.0.0 so trying to double check if its still emitting those metrics. Can you please confirm, if possible.

[breedx-splk]

Does OTel Java agent version-2.0 still emitting these metrics ?

I think it does but these are now named network.peer.port and server.port

It is renamed for the attribute net.sock.peer.addr → network.peer.address. net.sock.host.port is not defined and i think i am missing those metrics in v2.0.0 so trying to double check if its still emitting those metrics. Can you please confirm, if possible.

Yes, network.peer.address is still being emitted. https://github.com/open-telemetry/opentelemetry-java-instrumentation/blob/main/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/semconv/network/internal/InternalNetworkAttributesExtractor.java#L64

@trask Should we close this? Has it happened recently?

Seems like keeping it open might encourage folks to just continue asking about "metrics"...

[trask]

Hi @trask It seems, no breaking changes are mentioned specifically for net.sock.host.port attribute in either of those.

this was fixed by open-telemetry/semantic-conventions#804