Skip to content

Sending SQL from NpgsqlBatch to OpenTelemetry #5660

@tamas-nucleus

Description

@tamas-nucleus

I used OpenTelemetry data from Npgsql to debug a performance issue yesterday and was very happy with it. However, it seems that the SQL from NpgsqlBatches is not collected. Looking at the current code in main, it seems that the reason is that the CommandText property of the internal NpgsqlCommand of an NpgsqlBatch instance is null:

    internal NpgsqlCommand(int batchCommandCapacity, NpgsqlConnection? connection = null)
    {
        GC.SuppressFinalize(this);
        InternalBatchCommands = new List<NpgsqlBatchCommand>(batchCommandCapacity);
        InternalConnection = connection;
        CommandType = CommandType.Text;
        IsWrappedByBatch = true;

        // These can/should never be used in this mode
        _commandText = null!;
        _parameters = null!;
    }

It is the CommandText property that is used in the TraceCommandStart call:

    public override string CommandText
    {
        get => _commandText;
        set
        {
            Debug.Assert(!IsWrappedByBatch);
...
        }
    }

    internal void TraceCommandStart(NpgsqlConnector connector)
    {
        Debug.Assert(CurrentActivity is null);
        if (NpgsqlActivitySource.IsEnabled)
            CurrentActivity = NpgsqlActivitySource.CommandStart(connector, CommandText, CommandType);
    }

I find this strange, since the comment in the constructor said that the _commandText field should never be used, and yet it's returned directly from the property.

I was wondering if there was a reason for not sending SQL from NpgsqlBatch to OpenTelemetry, other than it's not implemented yet? Couldn't a simple solution be to change the CommandText property getter for NpgsqlCommands that are IsWrappedByBatch to return the CommandTexts of the contained NpgsqlBatchCommands concatenated? Or, if that's dangerous or has other side effects, to do such concatenation only when emitting telemetry? Would you accept a PR for that?

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

Projects

No projects

Milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions