[receiver/sqlquery] Add support for sybase database connections

[dehaansa]

Component(s)

receiver/sqlquery

Is your feature request related to a problem? Please describe.

I've seen requests from some users seeking to monitor their Sybase databases, and would like to utilize the sqlqueryreceiver for this purpose.

Describe the solution you'd like

There is a go implementation here of a driver for SAP ASE (formerly Sybase). It does not appear to be receiving a lot of active development but also does not have any significant issues reported.

Describe alternatives you've considered

There are prometheus exporters we've considered, but are interested in the opentelemetry ecosystem for this solution, and also looking for the ease of iteration offered by the query configuration in the sqlquery receiver.

Additional context

No response

[github-actions]

Pinging code owners:

• receiver/sqlquery: @dmitryax @crobert-1

See Adding Labels via Comments if you do not have permissions to add labels yourself.

[github-actions]

This issue has been inactive for 60 days. It will be closed in 60 days if there is no activity. To ping code owners by adding a component label, see Adding Labels via Comments, or if you are unsure of which component this issue relates to, please ping @open-telemetry/collector-contrib-triagers. If this issue is still relevant, please ping the code owners or leave a comment explaining why it is still relevant. Otherwise, please close it.

Pinging code owners:

• receiver/sqlquery: @dmitryax @crobert-1

See Adding Labels via Comments if you do not have permissions to add labels yourself.

[Grandys]

Hi @dehaansa , could you please assign me to this issue?

[dehaansa]

@Grandys that's above my permissions level, but I appreciate you taking it on!

[Grandys]

I've added experimental SAP ASE support in Go, but I need your input on a few issues I encountered:

• To run a SAP ASE database, I had to use the datagrip/sybase:16.0 image because version 15.7 fails to start on my machines (tested on both ARM and Intel MacBooks).
• When using the go-ase driver, I encountered an error (see issue #244) stating "expected TDS_MSG_SEC_ENCRYPT4, received TDS_MSG_SEC_ENCRYPT3 for ASE v16.0 SP04." By contrast, the thda/tds driver connects successfully. It’s worth mentioning that the thda/tds driver is distributed under the BSD license and appears to be less actively maintained (last commit 5 years ago versus 2 years ago for go-ase 😅).

My proposal is to:

• Include both drivers as dependencies so that clients can choose their preferred option.
• Update the documentation to indicate that users have two options for connecting to a SAP ASE database.
• Add integration tests exclusively for sybase:16.0 with the thda/tds driver.

Please let me know if this approach works for you or if you have any suggestions.

cc: @dmitryax @crobert-1

[crobert-1]

My preference would be to start with a single driver. Users shouldn't have to care about which golang sql driver is being used internally. If one ends up being a problem we can just swap it out with a bug fix and the user config stays the same. To me, this reduces dependencies and user confusion getting started.

I'm not a fan of either options of driver available to be honest. As you pointed out, both projects appear to be unmaintained, and not very widely used. It's at least encouraging that the thda/tds driver is connecting successfully, but my main concern would be potential vulnerabilities coming in from a small personal project that's no longer maintained. I guess we could rely on our vulnerability scanners for now.

My thought would be simply start with a new valid value for the driver config option, along with integration testing 👍

[Grandys]

Thank you for an update. One clarification:

... and the user config stays the same.

Replacing driver, is potentially breaking change, because connection string (in configuration) must change.
go-ase connection string is:

 ase://user:pass@host:port/

Where tds connection string is:

tds://username:password@host:port/database?parameter=value&parameter2=value2

[crobert-1]

Replacing driver, is potentially breaking change, because connection string (in configuration) must change.

Oh interesting, thanks for calling this out! I'd still prefer to start with one driver and add the second on an as-needed basis. In the future if we find one isn't working properly, or has security vulnerabilities that need to be avoided, a breaking change will be alright. I'm not sure how we could avoid a breaking change in either situation given the mismatched connection string. (If we included both drivers from the start we'd have the same breaking change if a security vulnerability popped up that required the removal of one of the drivers.)

That being said, maybe the driver UI name can be more closely related to the chosen driver, but still show that it's for SAP Sybase.

Is that a fair solution, do you have any concerns with the approach I've proposed?

[Grandys]

No concerns from my side, everything you wrote makes sense.

My PR is ready for review: #37773

[github-actions]

This issue has been inactive for 60 days. It will be closed in 60 days if there is no activity. To ping code owners by adding a component label, see Adding Labels via Comments, or if you are unsure of which component this issue relates to, please ping @open-telemetry/collector-contrib-triagers. If this issue is still relevant, please ping the code owners or leave a comment explaining why it is still relevant. Otherwise, please close it.

Pinging code owners:

• receiver/sqlqueryreceiver: @dmitryax @crobert-1

See Adding Labels via Comments if you do not have permissions to add labels yourself.

[dehaansa]

Closing as this was implemented in #37773