[feat] Add a request ID and include it in log lines

[daenney]

Description

This adds the necessary code to:

• Extend the global logger to allow for hooks that can be used to modify what's getting logged
• Add a new middleware that:
  • Fetches or generates a Request ID as the first middleware in the chain
  • Plonk the Request ID in both the Gin and stdlib context's
  • Add a hook to the logger to extract the RequestID and log it along with everything else
• Pull the context with our request ID through just about every log call so we can log it along with anything else

Checklist

Please put an x inside each checkbox to indicate that you've read and followed it: [ ] -> [x]

If this is a documentation change, only the first checkbox must be filled (you can delete the others if you want).

• I/we have read the GoToSocial contribution guidelines.
• I/we have discussed the proposed changes already, either in an issue on the repository, or in the Matrix chat.
• I/we have performed a self-review of added code.
• I/we have written code that is legible and maintainable by others.
• I/we have commented the added code, particularly in hard-to-understand areas.
• I/we have made any necessary changes to documentation.
• I/we have added tests that cover new code.
• I/we have run tests and they pass locally with the changes.
• I/we have run go fmt ./... and golangci-lint run.

[daenney]

Despite the fact that the request ID is now set on the context.Context and that there's a way to retrieve it, a lot of functions that may get called along the way from a request handler simply do log.Info() etc. calling into the global log package. This results in a log line without fields being emitted, so those lines don't emit an entry with a request ID.

If the log.XXX functions would take a context.Context as their first argument it becomes possible to extract a requestID in those functions and call logf with fields not set to nil.

[daenney]

So golang-ci lint is a little unhappy with us due to the context changes I guess. I'm not super convinced it's rightfully complaining about all cases though:

internal/media/cron.go:49:13: Non-inherited new context, use function like `context.WithXXX` instead (contextcheck)
			log.Error(nil, err)
			         ^
internal/media/processingmedia.go:251:33: Function `decodeVideoFrame->decodeVideoFrame$1` should pass the context parameter (contextcheck)
		video, err := decodeVideoFrame(rc)
		                              ^
internal/media/prune.go:89:14: Non-inherited new context, use function like `context.WithXXX` instead (contextcheck)
		if err := f(context.Background()); err != nil {
		           ^
internal/typeutils/astointernal.go:85:12: Non-inherited new context, use function like `context.WithXXX` instead (contextcheck)
		log.Infof(nil, "error extracting account emojis: %s", err)
		         ^
internal/typeutils/astointernal.go:260:43: Function `extractAttachments` should pass the context parameter (contextcheck)
	status.Attachments = c.extractAttachments(statusable)
	                                         ^
internal/config/validate.go:58:13: SA1012: do not pass a nil Context, even if a function permits it; pass context.TODO if you are unsure about which Context to use (staticcheck)
		log.Warnf(nil, "%s was set to 'http'; this should *only* be used for debugging and tests!", ProtocolFlag())
		          ^
internal/cache/util.go:38:13: SA1012: do not pass a nil Context, even if a function permits it; pass context.TODO if you are unsure about which Context to use (staticcheck)
	log.Panicf(nil, "failed %s after %d tries", msg, count)
	           ^
internal/concurrency/workers.go:69:12: SA1012: do not pass a nil Context, even if a function permits it; pass context.TODO if you are unsure about which Context to use (staticcheck)
	log.Infof(nil, "%s created with workers=%d queue=%d",

[daenney]

Thanks to the awesome help of @NyaaaWhatsUpDoc this is now ready to go! Ish. Once we figure out what to do about the lint thing.

[NyaaaWhatsUpDoc]

Definitely best to disable that linter imo. It's been annoying me for a while :p

[daenney]

Weird extra thing, the following lint warnings are bogus:

internal/media/processingmedia.go:251:33: Function `decodeVideoFrame->decodeVideoFrame$1` should pass the context parameter (contextcheck)
		video, err := decodeVideoFrame(rc)

internal/typeutils/astointernal.go:260:43: Function `extractAttachments` should pass the context parameter (contextcheck)
	status.Attachments = c.extractAttachments(statusable)

Neither of those two functions take a context in their signature.

[tsmethurst]

I'm OK with dropping the contextcheck linter altogether in a separate PR, afaik it hasn't found any actual hits yet, only false positives.

Anyway, reviewing this now :)

[tsmethurst]

looking good so far :)

[daenney]

The history is a bit messy now and you can't really roll back any individual commit so probably best to squash-merge this

[tsmethurst]

The history is a bit messy now and you can't really roll back any individual commit so probably best to squash-merge this

we always do, don't worry :)

[NyaaaWhatsUpDoc]

we love squerges

[daenney]

I smashed the history to make this easier to rebase. The only way I could find to retain @NyaaaWhatsUpDoc's authorship was the Co-authored-by tag in the commit. I hope that's OK.

[daenney]

I rebased this on main, should be good to go with 0.7.0 being out.

[tsmethurst]

Merged, thank you! :)