Add DB sql parsing to processor

[damemi]

Adds https://github.com/odigos-io/enterprise-go-instrumentation/pull/896 to the odigossqldboperationprocessor. This gives us coverage for all languages and shifts the parsing out of the agent

• Sets db.collection.name when available
• Sets span name to be {operation} {target} if both are available, or just {operation} if not (https://opentelemetry.io/docs/specs/semconv/database/database-spans/#name)
  • this only happens if span name is currently DB (so we are only modifying span names we created)

[damemi]

@RonFed fyi this happened switching to the vitess library:

$ make go-mod-tidy
go: finding module for package vitess.io/vitess/go/vt/sqlparser
go: downloading vitess.io/vitess v0.21.1
go: toolchain upgrade needed to resolve vitess.io/vitess/go/vt/sqlparser
go: vitess.io/vitess@v0.21.1 requires go >= 1.23.3; switching to go1.23.4
go: downloading go1.23.4 (darwin/arm64)
go: finding module for package vitess.io/vitess/go/vt/sqlparser
go: found vitess.io/vitess/go/vt/sqlparser in vitess.io/vitess v0.21.1

[RonFed]

I think we can remove the toolchain line, run go mod tidy again and it won't require the toolchain part but I'm not sure.

[RonFed]

Left some small comments.
I think we should do a load test with DB spans to make sure the CPU overhead of this processor is not too large.

[RonFed]

The DB name is specific to go auto instrumentation.
This creates some coupling. However, I don't have a suggestion for a better solution here since overriding the span name might not be desirable in a lot of cases.

[damemi]

exactly my thought, I'm assuming the other languages are able to follow the span name semconv easier? It would be nice if our default name from auto-instrumentation was something more indicative

[tamirdavid1]

Mike can u check the how it is effect the cpu on the gateway if exists? we can do it together of course

[damemi]

Seems like maybe there is a goroutine leak in the vitess library? Based on this failure that only showed up once I switched to it.

goleak: Errors on successful test run: found unexpected goroutines:
[Goroutine 19 in state select, with github.com/golang/glog.(*fileSink).flushDaemon on top of the stack:
github.com/golang/glog.(*fileSink).flushDaemon(0x19af1f8)
	/home/runner/go/pkg/mod/github.com/golang/glog@v1.2.2/glog_file.go:349 +0xb4
created by github.com/golang/glog.init.1 in goroutine 1
	/home/runner/go/pkg/mod/github.com/golang/glog@v1.2.2/glog_file.go:164 +0x126
]
FAIL	odigossqldboperationprocessor	0.452s
FAIL
make: *** [Makefile:62: test/./processors/odigossqldboperationprocessor/go.mod] Error 1

Or I'm not closing something properly with that library. I'll try to resolve that, then do the performance checks that @tamirdavid1 mentioned

[tamirdavid1]

Seems like maybe there is a goroutine leak in the vitess library? Based on this failure that only showed up once I switched to it.

goleak: Errors on successful test run: found unexpected goroutines:
[Goroutine 19 in state select, with github.com/golang/glog.(*fileSink).flushDaemon on top of the stack:
github.com/golang/glog.(*fileSink).flushDaemon(0x19af1f8)
	/home/runner/go/pkg/mod/github.com/golang/glog@v1.2.2/glog_file.go:349 +0xb4
created by github.com/golang/glog.init.1 in goroutine 1
	/home/runner/go/pkg/mod/github.com/golang/glog@v1.2.2/glog_file.go:164 +0x126
]
FAIL	odigossqldboperationprocessor	0.452s
FAIL
make: *** [Makefile:62: test/./processors/odigossqldboperationprocessor/go.mod] Error 1

Or I'm not closing something properly with that library. I'll try to resolve that, then do the performance checks that @tamirdavid1 mentioned

Here to help whatever needed

[damemi]

@tamirdavid1 @RonFed the memory leak looked like it was from glog, so I decided to look at #2038 to see if it could have just been a bug in the otel version pulled in by updating to vitess.

But, the more I think about it, I don't think we should use vitess. It's been problematic to get to work for several reasons:

• It links some Go runtime functions. This means we'll always have to be on the same Go version as vitess, which is a weird spot to tie ourselves to a dependency.
  • Technically this isn't done by the sqlparser package (it's vitess itself) but because that package isn't its own module we end up pulling in more of vitess than we need.
• This leads to my second point, that the vitess sql package doesn't seem like it's meant to be used as a standalone library. That means we don't get any compatibility or support guarantees and we're taking on a lot of risks by using it

I reverted the change to vitess, and I'm working on getting some profiling data now. @tamirdavid1 would you be able to get the profiling data to compare this branch now?

[github-actions]

This PR is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 30 days.

[github-actions]

This PR was closed because it has been stale for 30 days with no activity.