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

libgit2: Improved logging #776

Closed
wants to merge 0 commits into from
Closed

Conversation

pjbgf
Copy link
Member

@pjbgf pjbgf commented Jun 8, 2022

Debugging connection issues can be extremely difficult,
even more so at scale or when concurrent connections are
required to trigger specific issues.

The changes create an unique identifier for each
SubTransport instance, which allows for greater traceability
when going through all the steps throughout the transport
lifecicle.

Example of stdout output when grepping by a specific TransportID:

..."msg":"[ssh]: sshSmartSubtransport.Close()","server":"","TransportID":"64d598f7-62cd-4e23-b2ad-f4ec0350b254"...
..."msg":"[ssh]: sshSmartSubtransport.Close()","server":"source.developers.google.com:2022","TransportID":"64d598f7-62cd-4e23-b2ad-f4ec0350b254"...
..."msg":"[ssh] dial connection","server":"source.developers.google.com:2022","TransportID":"64d598f7-62cd-4e23-b2ad-f4ec0350b254"...
..."msg":"[ssh]: creating new ssh session","server":"source.developers.google.com:2022","TransportID":"64d598f7-62cd-4e23-b2ad-f4ec0350b254"...
..."msg":"[ssh]: run on remote","cmd":"git-upload-pack '/p/kubetestlocal/r/flux-testing'","server":"source.developers.google.com:2022","TransportID":"64d598f7-62cd-4e23-b2ad-f4ec0350b254"...
..."msg":"[ssh]: sshSmartSubtransport.Close()","server":"source.developers.google.com:2022","TransportID":"64d598f7-62cd-4e23-b2ad-f4ec0350b254"...
..."msg":"[ssh]: session.Close()","server":"source.developers.google.com:2022","TransportID":"64d598f7-62cd-4e23-b2ad-f4ec0350b254"...
..."msg":"[ssh] close client","server":"source.developers.google.com:2022","TransportID":"64d598f7-62cd-4e23-b2ad-f4ec0350b254"...
..."msg":"[ssh]: sshSmartSubtransport.Close()","server":"source.developers.google.com:2022","TransportID":"64d598f7-62cd-4e23-b2ad-f4ec0350b254"...
..."msg":"[ssh] close client","server":"source.developers.google.com:2022","TransportID":"64d598f7-62cd-4e23-b2ad-f4ec0350b254"...
..."msg":"[ssh]: sshSmartSubtransport.Close()","server":"source.developers.google.com:2022","TransportID":"64d598f7-62cd-4e23-b2ad-f4ec0350b254"...
..."msg":"[ssh] close client","server":"source.developers.google.com:2022","TransportID":"64d598f7-62cd-4e23-b2ad-f4ec0350b254"...

⚠️ This PR will be rebased once #775 is merged

@pjbgf pjbgf added the area/git Git related issues and pull requests label Jun 8, 2022
@pjbgf pjbgf added this to the GA milestone Jun 8, 2022
@@ -59,6 +59,7 @@ import (
"golang.org/x/crypto/ssh"
"golang.org/x/net/proxy"

"github.com/distribution/distribution/v3/uuid"
Copy link
Contributor

@darkowlzz darkowlzz Jun 8, 2022

Choose a reason for hiding this comment

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

Maybe we should use github.com/google/uuid instead? The whole package is specifically made for uuid and we already have it in our dependencies.

}, nil
}

type sshSmartSubtransport struct {
transport *git2go.Transport
uuid uuid.UUID
Copy link
Contributor

@darkowlzz darkowlzz Jun 8, 2022

Choose a reason for hiding this comment

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

Instead of adding a correlation ID here, we can make use of the perfect tool for it, context.WithValue(), and pass the ID in a context. I think it'd be in alignment with libraries like opentelemetry-go if we adopt them in the future.

@pjbgf
Copy link
Member Author

pjbgf commented Jun 9, 2022

Superseeded by #778

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/git Git related issues and pull requests
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants