parse sql host address and port

[esara]

parse sql host address and port for client spans to address #1254

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 71.66%. Comparing base (36ec19c) to head (a254148).

❗ There is a different number of reports uploaded between BASE (36ec19c) and HEAD (a254148). Click for more details.

HEAD has 1 upload less than BASE

Flag	BASE (36ec19c)	HEAD (a254148)
unittests	1	0

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1255      +/-   ##
==========================================
- Coverage   77.27%   71.66%   -5.62%     
==========================================
  Files         131      130       -1     
  Lines       13456    13456              
==========================================
- Hits        10398     9643     -755     
- Misses       2547     3174     +627     
- Partials      511      639     +128     

Flag	Coverage Δ
integration-test	61.19% <17.64%> (+0.02%)	⬆️
k8s-integration-test	57.37% <0.00%> (-1.22%)	⬇️
oats-test	36.40% <100.00%> (-0.72%)	⬇️
unittests	?

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[grcevski]

I really like the idea, I think we are almost there. I pulled this branch and I'm going to take a look at what it looks like to add this. I think the main issue is that ongoing_server_connections is only set for HTTP/gRPC server requests, but it will not have the outgoing DB information.

[grcevski]

Hi @esara, I added a PR (on your branch) which should address the missing connection info and will work even if the SQL was done on another Go routine. esara#14. If it works for you, after you accept the changes we can work on an integration test.

When I run this on the Go SQL test I added locally, it now shows something like:

2024-10-16 16:25:52.101642552 (527.375µs[527.375µs]) SQLClient 0 SELECT accounting.contacts [127.0.0.1 as 127.0.0.1:52456]->[127.0.0.1 as 127.0.0.1:5432] size:0B svc=[qosql go] traceparent=[00-bade493e98d04f9af022c4e64f3e31ba-b20d20f870017cfd[9a02c11b6f5334ca]-01]
2024-10-16 16:25:52.101642552 (1.17105ms[1.031505ms]) HTTP 200 GET /psqltest [::1 as ::1:41798]->[::1 as ::1:8080] size:0B svc=[qosql go] traceparent=[00-bade493e98d04f9af022c4e64f3e31ba-9a02c11b6f5334ca[0000000000000000]-01]

[esara]

Hi @esara, I added a PR (on your branch) which should address the missing connection info and will work even if the SQL was done on another Go routine. esara#14. If it works for you, after you accept the changes we can work on an integration test.

When I run this on the Go SQL test I added locally, it now shows something like:

2024-10-16 16:25:52.101642552 (527.375µs[527.375µs]) SQLClient 0 SELECT accounting.contacts [127.0.0.1 as 127.0.0.1:52456]->[127.0.0.1 as 127.0.0.1:5432] size:0B svc=[qosql go] traceparent=[00-bade493e98d04f9af022c4e64f3e31ba-b20d20f870017cfd[9a02c11b6f5334ca]-01]
2024-10-16 16:25:52.101642552 (1.17105ms[1.031505ms]) HTTP 200 GET /psqltest [::1 as ::1:41798]->[::1 as ::1:8080] size:0B svc=[qosql go] traceparent=[00-bade493e98d04f9af022c4e64f3e31ba-9a02c11b6f5334ca[0000000000000000]-01]

hi @grcevski this change looks great, I tested it locally with a slightly larger go application and a postgres backend and it is correctly detecting the sql service name and port and populates it in the client span

I am running more tests with the snowflake example, I mentioned in the issue later

[grcevski]

Hi Endre, I made a new PR with some integration tests esara#15. We've broken something so I made a draft PR so I can investigate the failrures myself. I'll push a new change to your PR once I know what's broken and close my PR.

[grcevski]

Hi Endre, I pushed new changes to my branch which is also used for my new PR against your branch and the tests passed: #1265. If you accept my PR changes as-is I think we should be in a good shape. I wonder if all these failures here might have to do with the version of clang/llvm you use for producing the BPF binaries? We are working towards switching to docker based builds for the BPF binaries to avoid this in the future.

[grcevski]

LGTM! Thanks again @esara for your contribution!