[sql] Implement updates to new semantic conventions for stored procedures#2693
Conversation
github-actions
Bot
added
the
comp:instrumentation.sqlclient
| activity.SetTag(SemanticConventions.AttributeDbOperationName, "EXECUTE"); | ||
| activity.SetTag(SemanticConventions.AttributeDbCollectionName, commandText); | ||
| activity.SetTag(SemanticConventions.AttributeDbQueryText, commandText); | ||
| activity.SetTag(SemanticConventions.AttributeDbStoredProcedureName, commandText); |
There was a problem hiding this comment.
Wanted to check with you all to confirm this meets the spirt of the recent changes regarding stored procedures.
- Removing
db.operation.name. I was arbitrarily setting it toEXECUTEbefore since there was no "operation" present in the query text. - Removing
db.collection.nameanddb.query.textsince their values would simply be the same asdb.stored_procedure.namemaking them redundant.
There was a problem hiding this comment.
Also, once I add db.query.summary should db.query.summary = db.stored_procedure.name?
There was a problem hiding this comment.
db.collection.name shouldn't be captured since it's not a collection
I would expect db.query.text to be captured if the stored proc is run via a query API (where you pass SQL), but from a non-query API where you explicitly pass a stored proc name
db.operation.name is a good question. I think if it's not a query API (where you pass SQL), then we should capture db.operation.name (but I suspect spec may not be clear about this)
There was a problem hiding this comment.
I don't think Java has a non query API for calling stored procedures
https://docs.oracle.com/javase/8/docs/api/java/sql/Connection.html#prepareCall-java.lang.String-
There was a problem hiding this comment.
The difference between using the "non-query" vs "query" API when invoking a stored procedure in C# is subtle.
"Non-query API"
This will be the more common way to invoke a stored procedure.
var command = new SqlCommand();
command.CommandType = CommandType.StoredProcedure;
command.CommandText = "some_stored_procedure"
command.Parameters.AddWithValue("@some_parameter", "some value");This is what I think you're proposing:
db.stored_procedure.name = some_stored_procedure
db.operation.name = EXECUTE or CALL or EXEC or ... maybe this is where the spec could be more clear
db.query.summary = EXECUTE some_stored_procedure (or CALL or EXEC)
db.query.text is not set
"Query API"
This will likely be less common, but still possible.
var command = new SqlCommand();
command.CommandType = CommandType.Text;
command.CommandText = "some_stored_procedure @some_parameter"
command.Parameters.AddWithValue("@some_parameter", "some value");I guess in this scenario we might only be able to set db.query.text.
db.stored_procedure.name is not set
db.operation.name is not set
db.query.text = some_stored_procedure @some_parameter
db.query.summary = ??? (it might be possible to parse the query text and safely infer that it is likely a stored procedure)
There was a problem hiding this comment.
thanks
In this case, should
db.query.summarybesome_stored_procedureor should it beexec some_stored_procedure?
I'm thinking it should be some_stored_procedure based on:
Instrumentations that support query parsing SHOULD parse the query and extract a
list of operations and targets from the query. It SHOULD setdb.query.summary
attribute to the value formatted in the following way:{operation1} {target1} {operation2} {target2} {target3} ...
There was a problem hiding this comment.
- not filed yet: add operation and collection name to SQL
now filed: open-telemetry/semantic-conventions#2207
There was a problem hiding this comment.
I'm thinking it should be some_stored_procedure based on:
hmmm. but the query sent to DB would be EXEC some_stored_procedure and operation.name would be EXEC, so summary should be EXEC some_stored_procedure.
There was a problem hiding this comment.
but the query sent to DB would be
EXEC some_stored_procedure
the way I understood the discussion above is that in this case:
var command = new SqlCommand();
command.CommandType = CommandType.Text;
command.CommandText = "some_stored_procedure"
the query sent to the DB would just be some_stored_procedure
There was a problem hiding this comment.
the query sent to the DB would just be
some_stored_procedure
Yup, that's correct.
|
This PR was marked stale due to lack of activity. It will be closed in 7 days. |
|
thanks all 🎉 |
6 participants
Implementing the new
db.stored_procedure.nameattribute.The
db.operation.name,db.collection.name, anddb.query.textattributes have been removed whenCommandType=StoredProcedurebecause they are no longer applicable.Side note, we have an odd scenario for .NET Framework users.
CommandTypeis not known, sodb.query.textwill still be used even for stored procedures. This requires documentation, but is beyond the scope of this PR.