-
Notifications
You must be signed in to change notification settings - Fork 3.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
HBASE-26473 Introduce db.hbase.container_operations
span attribute
#3951
HBASE-26473 Introduce db.hbase.container_operations
span attribute
#3951
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only a small question, not a blocker.
return ops; | ||
} | ||
|
||
public TableOperationSpanBuilder setContainerOperations( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we want to store a key value pair here to also reflect the cout for each operation?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's an interesting idea! What do you have in mind, something like db.hbase.container_operations.get.count
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a simple idea, not sure what is the best way to implement it. Checked the code, seems not easy to have a key value structure for an attribute? If we just encode it with a String, like 'Put,1000', seems it will make searching this attribute a bit harder? But if we introduce another attribute to store the count, we need to make these two attributes aligned, which is also a pain...
Anyway, not a big problem for now, could consider it later when we learn opentelemetry more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's not a bad idea. I don't now how the telemetry services will handle these kinds of fields.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's wait on this change until either we receive clear direction from open-telemetry/opentelemetry-specification#2202 or I can show clear benefit in my end-to-end testing.
return ops; | ||
} | ||
|
||
public TableOperationSpanBuilder setContainerOperations( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's not a bad idea. I don't now how the telemetry services will handle these kinds of fields.
Seems no progress on the otel side? So maybe it is a chance for us to define the best practice of tracing multi/batch operations? |
Still no comments from them. If you are still +1, @Apache9 , I'll rebase this commit and merge it. Lack of response from upstream isn't great though. |
Just go ahead. I'm OK with the current approach. |
For batch operations, collect and annotate the associated span with the set of all operations contained in the batch. Signed-off-by: Duo Zhang <zhangduo@apache.org>
af8bae7
to
ef97eec
Compare
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
For batch operations, collect and annotate the associated span with the set of all operations contained in the batch.
This is an HBase-specific extension that I am proposing. The idea here is to provide a single field containing all of the operations contained within an "envelope-type" operation. That field can then be indexed for query, allowing operators to easily find the operations that they seek. This is a project-specific proposal for a solution to the general issue I raised on open-telemetry/semantic-conventions#712.