support alternative way of skipping frames #60

pohly · 2021-08-04T07:49:26Z

This is needed for log sinks where the underlying stack unwinding
doesn't support an explicit call depth, for example testing.T.Log.

One such log sink is currently pending in kubernetes/klog#240.
It currently incorrectly logs logr.go as call site.

pohly · 2021-08-04T07:55:07Z

I'm just posting this for discussion. I still need to check whether this works as intended...

pohly · 2021-08-04T08:02:57Z

No, it doesn't 😢

Implementing the proposed Helper() LogSink runs into the same limitation as before:

func (l *tlogger) Helper() logr.LogSink {
	l.l.Helper()
	return l
}

It marks itself as helper, not its caller.

Looks like enhancements in the standard library will be necessary to address this. Scary.

pohly · 2021-08-04T09:05:31Z

This approach can be made to work by returning the underlying function that then needs to be called by the helper functions. I've pushed another commit for that and tested that it works for the testing logger.

However, the resulting API is definitely not nice and naming probably should be improved (too many "helpers"...).

logr.go

thockin

I patched this in and hacked on logr/testing and funcr and I can't make it work. The call to Helper() being in a func() means we're still off by one. Will look more later perhaps.

pohly · 2021-08-04T20:59:37Z

I've pushed further commits to make this work in the testing logger. With those changes I now get the expected result:

logr/testing $ go test -v .
=== RUN   TestTestLogger
    test_test.go:28: : "level"=0 "msg"="info"
    test_test.go:29: : "level"=0 "msg"="V(0).info"
    test_test.go:31: : "msg"="error" "error"="error"
    test_test.go:33: : "level"=0 "msg"="hello world"
--- PASS: TestTestLogger (0.00s)
PASS

thockin · 2021-08-05T02:14:26Z

funcr/funcr.go

 		prefix:    "",
 		values:    nil,
 		depth:     0,
 		write:     fn,
 		logCaller: opts.LogCaller,
 		verbosity: opts.Verbosity,
+		helper: opts.Helper,


gofmt should have lined these up?

Sorry for that. My gofmt integration in emacs sometimes breaks down without me noticing.

thockin

Looks OK overall.

Please squash and make sure the commit and PR messages are clean and helpful.
Can you please add the following file as funcr/example/main.go:

/*
Copyright 2021 The logr Authors.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

    http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package main

import (
	"fmt"

	"github.com/go-logr/logr"
	"github.com/go-logr/logr/funcr"
)

type E struct {
	str string
}

func (e E) Error() string {
	return e.str
}

func Helper(log logr.Logger, msg string) {
	helper2(log, msg)
}

func helper2(log logr.Logger, msg string) {
	log.WithCallDepth(2).Info(msg)
}

func main() {
	log := funcr.New(
		func(pfx, args string) { fmt.Println(pfx, args) },
		funcr.Options{LogCaller: funcr.All})
	example(log.WithValues("module", "example"))
}

// If this were in another package, all it would depend on in logr, not glogr.
func example(log logr.Logger) {
	log.Info("hello", "val1", 1, "val2", map[string]int{"k": 1})
	log.V(1).Info("you should see this")
	log.V(1).V(1).Info("you should NOT see this")
	log.Error(nil, "uh oh", "trouble", true, "reasons", []float64{0.1, 0.11, 3.14})
	log.Error(E{"an error occurred"}, "goodbye", "code", -1)
	Helper(log, "thru a helper")
}

thockin · 2021-08-05T02:20:53Z

logr.go

@@ -188,6 +191,13 @@ type Logger struct {
 	level         int
 	sink          LogSink
 	withCallDepth CallDepthLogSink
+
+	// withHelper is set in Init if the log sink supports the interface.


I'm going to be super nit-picky to avoid followups :)

// withHelper is set during init if the log sink supports the HelperLogSink // interface. // // We only need the function returned by withHelper.GetHelper(), but // cannot store it here because that makes Logger non-comparable, // which is something we want to guarantee.

logr.go

thockin · 2021-08-05T02:23:36Z

logr.go

@@ -261,6 +277,10 @@ func (l Logger) WithName(name string) Logger {
 // If the underlying log implementation supports a WithCallDepth(int) method,
 // it will be called and the result returned.  If the implementation does not
 // support CallDepthLogSink, the original Logger will be returned.
+//
+// Helper() should be used instead of WithCallDepth(1) because Helper()


Super nit - the () should be removed unless you are describing an actual call (e.g. WithCallDepth(1) is ok)

thockin · 2021-08-05T02:24:47Z

logr.go

@@ -269,6 +289,32 @@ func (l Logger) WithCallDepth(depth int) Logger {
 	return l
 }

+// Helper returns a Logger instance that skips the direct caller when


super nit: returns a new Logger instance

logr.go

thockin · 2021-08-05T02:30:26Z

logr.go

-	// specified number of frames when logging call site information.  If depth
-	// is 0 the attribution should be to the direct caller of this method.  If
-	// depth is 1 the attribution should skip 1 call frame, and so on.
+	// WithCallDepth returns a log sink that will offset the call stack by the


s/log sink/LogSink/

thockin · 2021-08-05T02:31:53Z

logr.go

 	// Successive calls to this are additive.
 	WithCallDepth(depth int) LogSink
 }
+
+// HelperLogSink represents a Logger that knows how to climb the call stack
+// to identify the original call site and can skip intermediate helper functions


Hard to tell but these lines look like thy might be > 80 columns? I have tried to keep comments wrapped cleanly

thockin · 2021-08-05T02:33:28Z

logr.go

+// intermediate helper functions.
+//
+// This is an optional interface and implementations are not required to
+// support it.


add "Implementations that choose to support this must not simply implement it as WithCallDepth(1), because Logger.Helper() will call both methods if they are present. This should only be implemented for LogSinks that actually need it, as with testing.T." or something like that?

Logger.Helper() or Logger.Helper? My chance for nit picking... 😀

I've added it with Logger.Helper.

thockin · 2021-08-05T02:53:19Z

funcr/funcr.go

@@ -55,6 +62,10 @@ type Options struct {
 	// Verbosity tells funcr which V logs to be write.  Higher values enable
 	// more logs.
 	Verbosity int
+
+	// Helper is an optional function that funcr must call to mark its own


s/must call/will call/

pohly · 2021-08-05T07:27:37Z

Looks OK overall.

So we are really doing this, despite the awkward API? Okay.

Please squash and make sure the commit and PR messages are clean and helpful.

Will do.

Can you please add the following file as funcr/example/main.go:

Okay, just one question about this:

// If this were in another package, all it would depend on in logr, not glogr.
func example(log logr.Logger) {

log.Info("hello", "val1", 1, "val2", map[string]int{"k": 1})
log.V(1).Info("you should see this")
    ...


I don't understand the comment. There's no reference to glogr in the file. Looks like a cut-and-paste problem, I'll simply drop the comment unless I hear otherwise.

thockin · 2021-08-05T14:44:48Z

The comment is a cut-paste error and can be removed, thanks

…

On Thu, Aug 5, 2021, 12:27 AM Patrick Ohly ***@***.***> wrote: Looks OK overall. So we are really doing this, despite the awkward API? Okay. Please squash and make sure the commit and PR messages are clean and helpful. Will do. Can you please add the following file as funcr/example/main.go: Okay, just one question about this: // If this were in another package, all it would depend on in logr, not glogr. func example(log logr.Logger) { log.Info("hello", "val1", 1, "val2", map[string]int{"k": 1}) log.V(1).Info("you should see this") ... I don't understand the comment. There's no reference to glogr in the file. Looks like a cut-and-paste problem, I'll simply drop the comment unless I hear otherwise. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#60 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABKWAVE6MBKQXS4KWEFODNLT3I4PHANCNFSM5BQPPGHA> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&utm_campaign=notification-email> .

thockin · 2021-08-05T16:22:38Z

So we are really doing this, despite the awkward API? Okay.

I don't love it, but given how testing.T,Helper() is implemented, I really don't see any other answer - the func HAS to be called from the helper-func's stack frame.

Before we merge this, I'll let it sit for a bit and see if anyone else has creative alternatives. I am doubtful, but maybe...

pohly

Force-pushed.

pohly · 2021-08-05T16:37:46Z

funcr/funcr.go

 		prefix:    "",
 		values:    nil,
 		depth:     0,
 		write:     fn,
 		logCaller: opts.LogCaller,
 		verbosity: opts.Verbosity,
+		helper: opts.Helper,


pohly · 2021-08-05T16:38:24Z

funcr/funcr.go

@@ -55,6 +62,10 @@ type Options struct {
 	// Verbosity tells funcr which V logs to be write.  Higher values enable
 	// more logs.
 	Verbosity int
+
+	// Helper is an optional function that funcr must call to mark its own


logr.go

pohly · 2021-08-05T16:39:22Z

logr.go

@@ -188,6 +191,13 @@ type Logger struct {
 	level         int
 	sink          LogSink
 	withCallDepth CallDepthLogSink
+
+	// withHelper is set in Init if the log sink supports the interface.


pohly · 2021-08-05T16:41:02Z

logr.go

@@ -269,6 +289,32 @@ func (l Logger) WithCallDepth(depth int) Logger {
 	return l
 }

+// Helper returns a Logger instance that skips the direct caller when


logr.go

pohly · 2021-08-05T16:47:27Z

logr.go

+// intermediate helper functions.
+//
+// This is an optional interface and implementations are not required to
+// support it.


Logger.Helper() or Logger.Helper? My chance for nit picking... 😀

I've added it with Logger.Helper.

thockin

LGTM. Great comments. Please squash to 1 commit and make sure commit and PR messages are complete.

We'll let this sit for a bit to see if anyone comes up with an obviously better answer.

@DirectXMan12 @wojas FYI

thockin · 2021-08-05T19:41:05Z

and if we merge this, I guess we'll want a 1.1.0 tag.

pohly · 2021-08-06T06:27:04Z

Please squash to 1 commit

I think that's too aggressive. 40bd6cd and 5bd8e58 are both stand-alone changes. We might even merge them already now, independently of the Helper API change. I'll put them into separate PRs so that we don't forget about them if we close this one here without merging.

Implementing the API and using it in testing can be squashed. I tend to prefer to keep that separate because it sometimes makes cherry-picking simpler, but that's subjective. Let's wait for more review feedback before I touch this PR again.

thockin · 2021-08-06T16:33:10Z

Fine, then :) Squash to the smallest set of whole commits that makes sense and make sure the comments are good ;)

wojas · 2021-08-09T08:09:19Z

func myWrapper(logger logr.Logger, msg string) {
    helper, logger := logger.Helper()
    helper()
    logger.Info(msg)
}

As an API this is horrible, but it does not look like we have much choice given how the Go testing package works.

Perhaps at least make the name on the logr side clearer. Given that this is a hack specifically added for the testing package, perhaps use something like WithTestingHelper instead? No need to pretend that this is some generic solution if this is really just a workaround for the testing package. Explicitly mentioning this in the comments would make it a less cryptic addition.

This would change the example as follows:

func myWrapper(logger logr.Logger, msg string) {
    helper, logger := logger.WithTestingHelper()
    helper()
    logger.Info(msg)
}

Would this be used in any other context?

Do I understand correctly that when not using the testing log sink, would you still need to use WithCallDepth? In that case you would need to use both this helper and WithCallDepth for a generic helper?

If this can be make more generic and completely replace the WithCallDepth call if no custom helper is provided, perhaps call it WithCallDepthHelper?

pohly · 2021-08-09T08:20:26Z

func myWrapper(logger logr.Logger, msg string) {
    helper, logger := logger.Helper()
    helper()
    logger.Info(msg)
}
As an API this is horrible, but it does not look like we have much choice given how the Go testing package works.

Perhaps at least make the name on the logr side clearer. Given that this is a hack specifically added for the testing package, perhaps use something like WithTestingHelper instead?

I agree that naming in the new API is sub-optimal (too much "helpers"). I just tried to get something working to get a feeling for how horrible it would be.

I like WithTestingHelper better (consistent with other Logger.With* functions, clearer reference to testing).

One concern I have is that users might get the impression that they should only use this when using testing. The function is also useful with other logging backends (see below).

No need to pretend that this is some generic solution if this is really just a workaround for the testing package. Explicitly mentioning this in the comments would make it a less cryptic addition.

Okay.

Would this be used in any other context?

I don't know of one. OTOH, that doesn't mean that there is no other logging backend that uses the same approach.

Do I understand correctly that when not using the testing log sink, would you still need to use WithCallDepth?
WithTestingHelper

No. A wrapper function must call either WithCallDepth(1) or WithTestingHelper(), but not both. Helper (= WithTestingHelper) is written so that it does the right thing for skipping the caller regardless of how the LogSink achieves that.

If this can be make more generic and completely replace the WithCallDepth call if no custom helper is provided, perhaps call it WithCallDepthHelper?

There's still a need for WithCallDepth when the parameter is greater than 1. Log wrappers which need that then don't work properly with testing.

wojas · 2021-08-09T08:33:24Z

OK, given that it calls WithCallDepth(1) if no helper is defined, it's basically a replacement for calling that directly and it should not have testing in the name.

I would then argue that calling something WithCallDepthHelper() would be the recommended way of implementing any helper function in your code. If you use this, it will work automatically for both testing loggers and for sinks that use the call depth approach. It also allows a log sink to implement to implement a similar mechanism as testing uses if it desires so.

We should then update the WithCallDepth documentation to recommend using the new helper approach instead.

Given the fragility of the API, perhaps add some extra checks there. It is too easy to forget to call the helper function and someone may find out that it 'still works' if you just ignore that return value with _. How about a descriptive panic() if the helper function was not called before using the new logger?

pohly · 2021-08-09T08:49:29Z

OK, given that it calls WithCallDepth(1) if no helper is defined, it's basically a replacement for calling that directly and it should not have testing in the name.

I would then argue that calling something WithCallDepthHelper() would be the recommended way of implementing any helper function in your code. If you use this, it will work automatically for both testing loggers and for sinks that use the call depth approach. It also allows a log sink to implement to implement a similar mechanism as testing uses if it desires so.

Correct. So replace Helper with WithCallDepthHelper?

We should then update the WithCallDepth documentation to recommend using the new helper approach instead.

That's already in the current PR 😄 ("Helper should be used instead of WithCallDepth(1) because ...")

Given the fragility of the API, perhaps add some extra checks there. It is too easy to forget to call the helper function and someone may find out that it 'still works' if you just ignore that return value with _. How about a descriptive panic() if the helper function was not called before using the new logger?

That would be nice, but cannot be implemented because the returned function intentionally doesn't go through the logger because testing.T.Helper cannot be wrapped, i.e. the logger cannot know whether it was called or not.

wojas · 2021-08-09T13:01:42Z

Correct. So replace Helper with WithCallDepthHelper?

That would be my preferred name choice. @thockin?

We should then update the WithCallDepth documentation to recommend using the new helper approach instead.

That's already in the current PR 😄 ("Helper should be used instead of WithCallDepth(1) because ...")

Ah, perfect, missed that. 😄

Given the fragility of the API, perhaps add some extra checks there. It is too easy to forget to call the helper function and someone may find out that it 'still works' if you just ignore that return value with _. How about a descriptive panic() if the helper function was not called before using the new logger?

That would be nice, but cannot be implemented because the returned function intentionally doesn't go through the logger because testing.T.Helper cannot be wrapped, i.e. the logger cannot know whether it was called or not.

Hmm, that's annoying. We could check it for situations where no helper was defined by the log sink and we return our own function, which could set some internal variable to false. But I don't like this inconsistency, because then you will get situations where code panics in production, even though it 'worked fine' in tests.

Perhaps just depend on Go's unused variable check then.

pohly · 2021-08-09T14:09:17Z

Perhaps just depend on Go's unused variable check then.

Yes. Hopefully programmers will think before discarding the helper function return value with _. What we would need to prevent that is golang/go#20803

thockin · 2021-08-09T19:06:14Z

I'm not 100% sure on names:

With*() implies some attribute of the Logger (WithName changes the Loggers name), which isn't really the case here. I'd expect WithCallDepthHelper to take an argument that was a "CallDepthHelper" and save that in the Logger.

Helper() has the advantage of following a convention (better than making something up). We don't really have examples of other implementations that are similar, so we don't know if there's actually a pattern to follow.

We're proposing that "best practice" be to call Helper() and run the returned function in (almost?) all cases of intermediate helper funcs. Given that, it seems to me that making it easier to use is important.

   log.Helper()()

vs

  log, helper := log.WithHelper()
  helper()

Which is more likely to be correct more often? The WithFoo() pattern is strongly associated with "catch the return value", where as t.Helper() is strongly associated with fire-and-forget. OTOH 1 line vs 2 lines and named variables.

Thoughts?

pohly · 2021-08-09T19:18:03Z

Helper() has the advantage of following a convention (better than making something up).

You mean testing.T.Helper? That's exactly the fire-and-forget semantic that we don't want to follow here because the return values must both be used. For me that's a strong argument to use With* because then using the return value is a familiar pattern.

I agree that WithCallDepthHelper sounds like it should have a parameter. How about WithOneMoreCallDepth? "One" implies that the parameter is not needed because it would be 1 and "More" implies that it is additive. It rolls of the tongue better than WithOneCallDepth, too.

wojas · 2021-08-10T02:21:55Z

The main issue with log.Helper()() is that this way we cannot increment the call depth for non-testing backends. It would the require every helper to both call log.Helper()() and WithCallDepth, the second one returning a new instance like the proposed WithCallDepthHelper or whatever the name ends up being:

Compare

func myWrapper(logger logr.Logger, msg string) {
    helper, logger := logger.WithCallDepthHelper()
    helper()
    logger.Info(msg)
}

with

func myWrapper(logger logr.Logger, msg string) {
    logger.Helper()()
    logger.WithCallDepth(1).Info(msg)
}

And as @pohly mentioned, the Go compiler will also not help you if you just call logger.Helper() by mistake without using the returned function, see https://play.golang.org/p/lfwo09sbGkh

thockin · 2021-08-10T15:54:54Z

OK. I had missed that, of course, we can't just call WithCallDepth() internally without returning the new logger.

So let's talk about 2 things:

logger, helper := ... vs helper, logger := ...
Names. Let's brainstorm and see if anything obvious emerges.

x, y := log.AsHelper()

x, y := log.AsHelperFunc()

x, y :=log.IsHelper()

x, y := log.ThisIsAHelperFunc()

x, y := log.GetHelperFunc()

more welcome...

pohly · 2021-08-10T18:18:31Z

logger, helper := ... vs helper, logger := ...

I don't have a strong opinion either way. At the end, both is arbitrary.

Regarding names:

My understand is that in testing, "the helper" is the log wrapper function. testing.T.Helper is just that API for marking the helper function. I wouldn't call that method "the helper", therefore a -1 from me for GetHelperFunc. I don't have a good suggestion for naming the variable, though. For consistency with t.Helper() in normal test code I probably would stick with helper().

All of other examples might look okay at first glance when called without using the return values although such usage is incorrect. Therefore I still prefer WithOneMoreCallDepth.

thockin · 2021-08-10T19:42:09Z

WithOneMoreCallDepth really doesn't "feel good" to me. I'm not really looking to rush this, so I'll let it percolate for a bit.

wojas · 2021-08-11T13:45:31Z

logger, helper := ... vs helper, logger := ...

Perhaps logger, helper := is better, because the context package also does this:

func WithCancel(parent Context) (ctx Context, cancel CancelFunc)

x, y := log.AsHelper()
x, y := log.AsHelperFunc()

To me As suggests a conversion, but we do not only return the cancel function and there is no actual conversion happening.

x, y :=log.IsHelper()

Feels like a check if this is a helper. I would expect a bool return value.

x, y := log.ThisIsAHelperFunc()

Suggests that this actual call marks it as a helper func. I would also not expect any return value here.

x, y := log.GetHelperFunc()

This would be fine is it would just return a helper func.

I think something starting with With would still be most clear. This is what we do with WithCallDepth and the pattern mirrors context.WithCancel. It suggests that you are adding something to the current object and receiving a new object.

@pohly is right that the calling function is the helper in the testing naming, but I think the function can still be seen as a helper to determine that call stack depth to subtract.

I still have a preference for WithCallDepthHelper, because like in WithCancel, it tells something about the extra return value.
WithOneMoreCallDepth does not tell you anything about the weird extra return value.

Come to think of it, this is a pretty common pattern:

ctx, cancel := context.WithCancel(ctx)
defer cancel()

This does not look too weird next to that:

logger, helper := logger.WithCallDepthHelper()
helper()

thockin · 2021-08-11T15:27:00Z

I guess I buy that reasoning for With*.

WithCallDepthHelper()

WithCallStackHelper()

WithCallerHelper()

WithHelper()

wojas · 2021-08-12T04:41:22Z

One more:

WithCallDepthMarkerFunc()

thockin · 2021-08-13T16:59:37Z

rebase needed :)

pohly · 2021-08-17T06:30:51Z

rebase needed :)

Can you merge PRs #61 and #62 first? Then a rebase will make this PR smaller.

thockin · 2021-08-17T16:56:25Z

Done

pohly · 2021-08-18T16:05:33Z

@wojas: @thockin and I chatted about this a bit more and agreed on WithCallStackHelper because WithCallDepthHelper sounds like it should have a numeric depth argument, similar to WithCallDepth. I'll rename accordingly.

thockin · 2021-08-18T16:45:22Z

logr.go

+type CallStackHelperLogSink interface {
+	// GetHelper returns a function that must be called to mark the direct caller
+	// as helper function when logging call site information.
+	GetHelper() func()


should this be GetHelperFunc() or GetCallStackHelper() ?

I wasn't sure. If we want a longer name, then I prefer GetCallStackHelper because it is more consistent.

I've pushed a version with GetCallStackHelper.

This is needed for log sinks where the underlying stack unwinding doesn't support an explicit call depth, for example testing.T.Log. Users should use the new logger.Helper method whenever possible by calling it in each log helper function because it also works with LogSinks that support either the CallDepthLogSink interface or the new HelperLogSink interface, whereas Logger.WithCallDepth only works with CallDepthLogSink. A logger might implement both if it supports two different ways of finding source code. For example, testing/test.go optionally unwinds itself and then also testing.T adds source code information.

The funcr log sink must implement the new CallStackHelperLogSink interface (otherwise the wrapper function wouldn't be handled correctly) and also mark its own wrapper as a helper.

thockin · 2021-08-18T18:45:35Z

LGTM

pohly closed this Aug 4, 2021

pohly mentioned this pull request Aug 4, 2021

RFC: testinglogger: per-test, structured logging kubernetes/klog#240

Closed

pohly reopened this Aug 4, 2021

thockin reviewed Aug 4, 2021

View reviewed changes

logr.go Show resolved Hide resolved

thockin reviewed Aug 4, 2021

View reviewed changes

thockin reviewed Aug 5, 2021

View reviewed changes

pohly force-pushed the helper-call-skip branch from e94d002 to 5bd8e58 Compare August 5, 2021 18:44

pohly commented Aug 5, 2021

View reviewed changes

thockin reviewed Aug 5, 2021

View reviewed changes

This was referenced Aug 6, 2021

funcr: add example #61

Merged

CallDepthLogSink: clarify relationship with RuntimeInfo #62

Merged

pohly mentioned this pull request Aug 11, 2021

export Logger.Sink #66

Merged

pohly force-pushed the helper-call-skip branch from 5bd8e58 to 0ebd759 Compare August 18, 2021 16:23

pohly changed the title ~~RFC: support alternative way of skipping frames~~ support alternative way of skipping frames Aug 18, 2021

pohly force-pushed the helper-call-skip branch from 0ebd759 to 742e83f Compare August 18, 2021 16:38

thockin reviewed Aug 18, 2021

View reviewed changes

pohly added 2 commits August 18, 2021 19:15

testing: fix source code location printed by testing.T

e16cd87

The funcr log sink must implement the new CallStackHelperLogSink interface (otherwise the wrapper function wouldn't be handled correctly) and also mark its own wrapper as a helper.

pohly force-pushed the helper-call-skip branch from 742e83f to e16cd87 Compare August 18, 2021 17:16

thockin merged commit cf509d1 into go-logr:master Aug 18, 2021

pohly mentioned this pull request Aug 19, 2021

embed funcr, custom With* function example #68

Merged

support alternative way of skipping frames #60

support alternative way of skipping frames #60

Conversation

pohly commented Aug 4, 2021

pohly commented Aug 4, 2021

pohly commented Aug 4, 2021

pohly commented Aug 4, 2021

thockin left a comment

Choose a reason for hiding this comment

pohly commented Aug 4, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thockin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pohly commented Aug 5, 2021

thockin commented Aug 5, 2021 via email

thockin commented Aug 5, 2021 • edited Loading

pohly left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thockin left a comment • edited Loading

Choose a reason for hiding this comment

thockin commented Aug 5, 2021

pohly commented Aug 6, 2021

thockin commented Aug 6, 2021

wojas commented Aug 9, 2021

pohly commented Aug 9, 2021 • edited Loading

wojas commented Aug 9, 2021

pohly commented Aug 9, 2021

wojas commented Aug 9, 2021

pohly commented Aug 9, 2021

thockin commented Aug 9, 2021

pohly commented Aug 9, 2021

wojas commented Aug 10, 2021 • edited Loading

thockin commented Aug 10, 2021

pohly commented Aug 10, 2021

thockin commented Aug 10, 2021

wojas commented Aug 11, 2021

thockin commented Aug 11, 2021

wojas commented Aug 12, 2021

thockin commented Aug 13, 2021

pohly commented Aug 17, 2021

thockin commented Aug 17, 2021

pohly commented Aug 18, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thockin commented Aug 18, 2021

thockin commented Aug 5, 2021 •

edited

Loading

thockin left a comment •

edited

Loading

pohly commented Aug 9, 2021 •

edited

Loading

wojas commented Aug 10, 2021 •

edited

Loading