[receiver/sqlserver] Allow server config option to include port

[crobert-1]

Component(s)

receiver/sqlserver

Describe the issue you're reporting

Current functionality

The functionality of directly connecting to a SQL Server instance was recently added, with the configuration options of server and port as two distinct options. I mentioned this as a possible pain point, and now I've run into it myself.

Just for context, the use case where I'd like to specify port in server is for using with the observer extensions and receiver creator receiver. The observer extensions only pass in the endpoint to the receiver creator as a variable that can be dynamically set in the receiver configuration. From documentation, this endpoint will sometimes also include the port. If the port is included, there's no way to currently use the SQL server receiver with the receiver creator.

Proposal

Allow the server option to optionally have the port included. If the port is included in the server option, it will override the given port value. The port will still need to be specified in one option or the other, or config validation will fail.

This is simply an enhancement, no existing configurations should be impacted.

Examples

# Port used to connect to SQL Server instance will be 1433
server: 0.0.0.0
port: 1433

# Port used to connect to SQL Server instance will be 1433
server: 0.0.0.0:1433

# Port used to connect to SQL Server instance will be 1433
server: 0.0.0.0:1433
port: 9898

# Config validation will fail
server: 0.0.0.0

[github-actions]

Pinging code owners:

• receiver/sqlserver: @djaglowski @StefanKurek

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

[djaglowski]

IMO, this seems like a problem with the observer extension's behavior. The proposed to this receiver introduces ambiguity into the configuration. Are you sure there isn't a reasonable change we could make to the observer package instead?

[crobert-1]

Fair point, and I agree. It does make the configuration a bit more confusing.

My original concern here was the extent of changes required in the observer extension, as I wasn't very familiar with its inner-workings. After more of a look though, it appears to be a pretty simple change to be able to expose the values we need in the extension, so I think I'll move my efforts there instead. 👍