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

Logging Handler #31

Closed
Tracked by #1
jobala opened this issue Jan 24, 2022 · 5 comments · Fixed by microsoft/kiota-http-go#29
Closed
Tracked by #1

Logging Handler #31

jobala opened this issue Jan 24, 2022 · 5 comments · Fixed by microsoft/kiota-http-go#29
Assignees
Labels
enhancement New feature or request

Comments

@jobala
Copy link
Contributor

jobala commented Jan 24, 2022

https://github.com/microsoftgraph/msgraph-sdk-design/blob/master/middleware/LoggingHandler.md

@mbarnes
Copy link

mbarnes commented Apr 13, 2022

I came up with my own solution to this while debugging msgraph-sdk-go error responses.

It does use a third-party module but a simple one (github.com/motemen/go-loghttp).

I offer it here in case it's of any value.

import (
        "log"
        "net/http"
        "net/http/httputil"

        msgraph "github.com/microsoftgraph/msgraph-sdk-go"
        msgraphcore "github.com/microsoftgraph/msgraph-sdk-go-core"
        loghttp "github.com/motemen/go-loghttp"
)

...

	// Add logging to the client for debugging.
	client := msgraphcore.GetDefaultClient(nil)
	client.Transport = &loghttp.Transport{
		Transport: client.Transport,
		LogRequest: func(req *http.Request) {
			if dump, err := httputil.DumpRequestOut(req, true); err == nil {
				log.Printf("%s", string(dump))
			}
		},
		LogResponse: func(resp *http.Response) {
			if dump, err := httputil.DumpResponse(resp, true); err == nil {
				log.Printf("%s", string(dump))
			}
		},
	}

	adapter, err := msgraph.NewGraphRequestAdapterWithParseNodeFactoryAndSerializationWriterFactoryAndHttpClient(authProvider, nil, nil, client)

@baywet
Copy link
Member

baywet commented Apr 13, 2022

Hi @mbarnes ,
Thanks for sharing this.
Providing the ability to log request and responses has always been something we've debated a lot internally.
On one hand it can be really useful to debug.
On the other, somebody could easily forget to remove it and ship it in production, logging sensible data.
A middle ground could be to enable conditional compilation so the code is only present for debug builds. But I'm not sure it's a thing in go like it is in dotnet.
Regardless we have our architect working on an observability spec (tracing, logging and metrics) for all the kiota based sdks.

@baywet
Copy link
Member

baywet commented Apr 13, 2022

microsoft/kiota#1486

@mbarnes
Copy link

mbarnes commented Apr 13, 2022

Yeah, conditional compilation isn't a thing in Go unfortunately.
In my case, I'm using an environment variable to enable the request/response logging.

@baywet
Copy link
Member

baywet commented Apr 13, 2022

A good alternative. We could also have the handler as part of the http package but not of the default handlers that are added to the client to drive enable/disable.
And when the handler is added, log very loud warnings so people notice it if enabled in production.
These are the kind of discussions we've had so far without reaching a consensus.
Cc @darrelmiller in case you want to revisit as part of your observability spec.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
3 participants