add lagerctx package

[xoebus]

Passing both a Context and Logger into a lot of functions was pretty exasperating. I'm looking for feedback on this new package to avoid the bloat.

Let me know what you think!

[cfdreddbot]

Hey xoebus!

Thanks for submitting this pull request! I'm here to inform the recipients of the pull request that you and the commit authors have already signed the CLA.

[cf-gitbot]

We have created an issue in Pivotal Tracker to manage this:

https://www.pivotaltracker.com/story/show/145647699

The labels on this github issue will be updated when the story is started.

[xoebus]

I switched over a component of our service to use this. I'm pretty happy with how it turned out. The interface looks far cleaner now.

pivotal-cf/cred-alert@2d26a85

[emalm]

Thanks, @xoebus ! Looks pretty reasonable to me, but you can't trust me nowadays, so let's have a pair from Diego look over it too.

I'm not sure of the community conventions for context-stored types and their null or default values, but would it make sense to export the discardLogger type? I could see it might be useful to have the type available in a called routine to determine whether the Logger extracted from the routine's context is this default, no-op one and, if so, then to replace it with a minimally functional stdout logger and to propagate that in the context to its own callees.

Thanks,
Eric

[xoebus]

I'm following a pattern set forward by the cloud.google.com/go/trace package. It does not perform any tracing if there is no trace attached to a Context. The discardLogger could be exposed but it is not required for the feature and this avoids expanding the API of the package. It can always be exported later.

I'm not a fan of consumers of this package needing to reach inside the context to understand what kind of logger they're getting back. They should just be treating it as them being given something which corresponds to the lager.Logger interface which they can log to. It is the job of the caller to make sure that it's calling it's dependencies with everything they need in order to operate. The discardLogger is just a safe default.

[xoebus]

I've been thinking of adding a new function to lagertest which does something like:

func NewContext(parent context.Context, name string) context.Context {
    return lgctx.NewContext(parent, NewTestLogger(name))
}

Maybe also allowing the parent context to be nil?

[crhino]

@xoebus, we looked the PR over and it looks good to us. We agree that there is no need to export discardLogger currently.

To bikeshed slightly, we thinking about whether lgctx is the right now for the package. Perhaps lagerctx might be more in line with the other packages provided by lager, such as lagertest and lagerflags?

@chrino && @anoop2811

[xoebus]

I chose brevity here because this package name will be appearing quite a lot of times throughout a codebase if it is used. There's some prior art here in the standard library e.g. fmt.

[xoebus]

I've updated the PR with the new package name.

Does Lager need to work on Go versions before 1.7?

[jvshahid]

per @ematpl it is fine to only support go 1.7 and later (link to the slack conversation)