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

Add tracing to txn request. #11191

Closed
YoyinZyc opened this issue Oct 1, 2019 · 5 comments
Closed

Add tracing to txn request. #11191

YoyinZyc opened this issue Oct 1, 2019 · 5 comments

Comments

@YoyinZyc
Copy link
Contributor

YoyinZyc commented Oct 1, 2019

Adding tracing to txn request is kind of different from to range or put request. A txn request could have couple of range, put, delete and txn operations. I think it will be confusing to just put all of these operations' traces together. My first idea is to use subtrace for each operations. The basic structure is like this:

...
"msg": "trace[12345] transaction", "details": "...", "duration":"100ms", "start":"...", "end":"...",
"steps": [
trace[12345] 'compare' (duration: 1ms),
trace[12345]: "msg": "subtrace range", "details": "...", "duration":"20ms", "start":"...", "end":"...", "steps": [...],
trace[12345]: "msg": "subtrace put", "details": "...", "duration":"20ms", "start":"...", "end":"...", "steps": [...],
...
]

The potential concern on this approach is that the nested trace is not quite friendly with structure logging.
My second thought is adding the sub requests' information into step's detailed fields.

...
"msg": "trace[12345] transaction", "details": "...", "duration":"100ms", "start":"...", "end":"...",
"steps": [
trace[12345] 'compare' (duration: 1ms),
trace[12345]: "agreement among raft nodes before linearized reading {"type":"range", "range_start":"foo",...} duration: 20ms",
...
trace[12345]: "raft request"{"type":"put", "key":"fooo"...} duration:1ms",
...
]

The drawback of this one is that it will increase the volume of log and may be confusing especially when we have nested transactions.
Do you have some suggestions on the arrangement of txn tracing?

@YoyinZyc
Copy link
Contributor Author

YoyinZyc commented Oct 1, 2019

@jpbetz @gyuho @jingyih Any suggestions?

@jpbetz
Copy link
Contributor

jpbetz commented Oct 1, 2019

My general preference would be that we retain as much structure as possible and try to avoid putting structured data into strings via some serialization. I don't know zap that well, unfortunately. What does zap.Any do if we provide a struct that models our desired tracing output?

@gyuho
Copy link
Contributor

gyuho commented Oct 1, 2019

@YoyinZyc @jpbetz Or just simply tag with level (zap.Int("tracing-level", 1))? Regarding Txn`, it will be level 1, and subsequent puts and ranges can be level 2. What do you think

@gyuho
Copy link
Contributor

gyuho commented Oct 1, 2019

Oh wait, it will be traced in the same message, so we would need better way to differentiate within the message.

@gyuho gyuho modified the milestones: etcd-v3.4, etcd-v3.5 Oct 3, 2019
@stale
Copy link

stale bot commented Apr 6, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Apr 6, 2020
@stale stale bot closed this as completed Apr 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

3 participants