Skip to content
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

CURATOR-726: Improve tracing for certain operations #513

Merged
merged 5 commits into from
Dec 20, 2024

Conversation

HoustonPutman
Copy link
Contributor

https://issues.apache.org/jira/browse/CURATOR-726

In Apache Solr, we are trying to refactor all of our custom ZK interaction logic to use Curator. One of the things we do is implement metrics on our interactions with ZK. Curator allows us to do this with Tracing and CuratorListeners. However there are two things that don't give us the information we need.

  1. The "Get Children" command trace does not give the information needed for the number of Children fetched.
  2. The "Multi Transaction" command trace:
    • The trace does not give the number of commands that are in the transaction, which is something we track currently.
    • The MultiTransaction trace is not an AdvancedTrace, it's a normal Trace for some reason. AdvancedTracerDrivers cannot read regular traces, so we can't even see these. We need to update Curator to use an AdvancedTrace for this command.

Therefore in this PR, I have added the two pieces of information (# of Children fetched and # of transactions) and also converted the MultiTransaciton to use an AdvancedTrace.

Copy link
Member

@tisonkun tisonkun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@tisonkun tisonkun requested a review from kezhuw December 18, 2024 03:27
Copy link
Member

@kezhuw kezhuw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

I think we probably should trace also opcode in future. The name is not good for programmatical decision.

@HoustonPutman
Copy link
Contributor Author

I think we probably should trace also opcode in future. The name is not good for programmatical decision.

100% agreed! CuratorEventType already exists, so we can repurpose it for this as well.

@HoustonPutman
Copy link
Contributor Author

I think we probably should trace also opcode in future. The name is not good for programmatical decision.

100% agreed! CuratorEventType already exists, so we can repurpose it for this as well.

Well that's very frustrating. CuratorEventType is in curator-framework, while OperationTrace is in curator-client, so OperationTrace cannot have a CuratorEventType variable...

@kezhuw
Copy link
Member

kezhuw commented Dec 19, 2024

I think we probably should trace also opcode in future.

I guess this require a bunch of changes and worth a separated issue. Let's leave it for future improvement.

@kezhuw kezhuw merged commit eb99124 into apache:master Dec 20, 2024
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants