Activity (OpenTelemetry) for DataReader

[jods4]

I would love to have an activity span that starts when a DataReader is created and stops when it's closed, and that includes the number of read rows.

It is useful as fetching/processing can take significant time for some queries. Knowing when ExecuteQuery completed is only a small part of the perf profile. Knowing how many rows a query returns is also quite useful.

If you want some context/inspiration, I had added this myself when I was patching older ODP.NET releases to add ActivitySource into the provider: https://github.com/jods4/OracleApmPatcher/blob/master/Src/Program.cs#L288

I have seen that the DataReader fetches (network round-trips) are instrumented, which is not exactly the same. It could be an alternative, if you add the actual number of rows fetched, and ideally an activity when the reader is closed.

BTW: why do you parse and re-construct SQL text in SpanStart? Why is it not enough/okay to receive the actual text that was executed? This is performed on each query and allocates a lot of strings. I would love an option to get the raw, unprocessed, query text.

[jods4]

One more thing: please make OracleConfiguration.EnableOpenTelemetry() public.
Currently to enable it we need the Oracle.ManagedDataProvider.OpenTelemetry package and we must provide a TraceBuilder.
ActivitySource is useful in itself even without OpenTelemetry... we send APM to a different (proprietary) system.

[alexkeh]

@jods4 It sounds like you have four requests:

1. An activity span that starts when a DataReader is created and stops when it's closed.
   A DataReader can read multiple result sets in its lifetime. Do you mean you want the activity span to start when it begins reading a result set until the end of the result set? It can then start a new activity span if the NextResult() method is called. This would cover retrieval time for each result set.

2. Number of rows the DataReader reads.
   Are you looking for db.response.returned_rows? This attribute covers the rows fetched from the DB for a specific operation.

3. Actual query text executed.
   ODP.NET has two activity tags to track the statements being executed: db.statement for the DB statement being executed and db.odp.user.statement for the user-supplied command text. It sounds like you want to use the latter. One thing to keep in mind is that we redact literal values, which can have PII or other sensitive info that should never appear in a trace.

4. OracleConfiguration.EnableOpenTelemetry() to be public.
   Let me check with the ODP.NET OTel dev team on this option.

[jods4]

A DataReader can read multiple result sets in its lifetime. Do you mean you want the activity span to start when it begins reading a result set until the end of the result set? It can then start a new activity span if the NextResult() method is called. This would cover retrieval time for each result set.

That's a good point. (It's not something I often use myself).
I would agree one activity per ResultSet is better!

Are you looking for db.response.returned_rows? This attribute covers the rows fetched from the DB for a specific operation.

Yes, looks like this standard tag is usable for that purpose.

ODP.NET has two activity tags to track the statements being executed: db.statement for the DB statement being executed and db.odp.user.statement for the user-supplied command text. It sounds like you want to use the latter.

Let me double-check next week, it looks like I might have missed something here. Or is there an option or a specific step to take to get db.odp.user.statement? I think missed it somehow.

One thing to keep in mind is that we redact literal values, which can have PII or other sensitive info that should never appear in a trace.

I agree, tracing/logging as it can contain sensitive info, so it's good that you offer safe-by-default options.
That being said, it has drawback in perf and usability (can't just copy-paste requests to execute them for debugging purposes) and in our situation at least it's not required: we never put sensitive info inside these queries (at least not in clear text), and we're capturing the traces into Elastic APM which has its own flexible rules based on patterns to anonymize captured information.
I think it would make sense to have an option to opt out of SQL redaction.

[alexkeh]

The db.odp.user.statement tag is instrumented only when the following two conditions are met:

1. The internally executed statement is different from the user-supplied command text
2. setDBStatementForText or SetDbStatementForStoredProcedure is set to true, depending on which is applicable, just like db.statement tag.

I'll follow up with the dev team to answer your other questions.

[jods4]

Thanks for the explanation. In my case SetDbStatementForText is true (I wouldn't have db.statement either otherwise) but I suppose the user and internal commands are the same.

That being said, I observe that db.odp.user.statement go through the same redaction process (Parse + ReconstructSQL). So db.odp.user.statement is not really what I would like. What I would like is an option to disable the SQL redaction (from both tags).