Skip to content
This repository has been archived by the owner on Aug 12, 2022. It is now read-only.

feat: More logging around execute/multiplex #233

Closed
wants to merge 6 commits into from
Closed

Conversation

disq
Copy link
Member

@disq disq commented Apr 27, 2022

No description provided.

@disq disq requested a review from roneli April 27, 2022 13:23
@github-actions github-actions bot added the feat label Apr 27, 2022
Copy link
Contributor

@spangenberg spangenberg left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@roneli roneli left a comment

Choose a reason for hiding this comment

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

Some thoughts around the added logging, I want to look at the execution also to see if any more lines to suggest.

provider/execution/execution.go Outdated Show resolved Hide resolved
provider/execution/execution.go Show resolved Hide resolved
provider/execution/execution.go Show resolved Hide resolved
@yevgenypats
Copy link
Member

Im not sure I understand how this help pair start with end logs around critical points. I would try to test it and then run a regex or a python script to make sure we are able to catch those lines. Otherwise we will have long roundtrips with the customer

@disq
Copy link
Member Author

disq commented Apr 28, 2022

@roneli I tried the withLogger approach but it makes things very implicit and confusing.

@disq
Copy link
Member Author

disq commented Apr 28, 2022

or #235

@disq disq closed this Apr 29, 2022
@disq disq deleted the feat/more-logging branch May 2, 2022 14:01
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants