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

Feature: Make initStandaloneModeLogger public #105

Closed
alarbada opened this issue Feb 7, 2024 · 1 comment
Closed

Feature: Make initStandaloneModeLogger public #105

alarbada opened this issue Feb 7, 2024 · 1 comment
Labels
feature New feature or request wontfix This will not be worked on

Comments

@alarbada
Copy link
Contributor

alarbada commented Feb 7, 2024

Feature description

I first thought that it would be useful to allow the acceptance tests to have a custom context. But after digging further on why didn't a sdk.Logger(ctx).Info() call log anything, I found out that the used zerolog instance is the disabled one.

Specifically, this:

func initStandaloneModeLogger() error {
	// adjust field names to have parity with hclog, go-plugin uses hclog to
	// parse log messages
	zerolog.LevelFieldName = "@level"
	zerolog.CallerFieldName = "@caller"
	zerolog.TimestampFieldName = "@timestamp"
	zerolog.MessageFieldName = "@message"

	logger := zerolog.New(os.Stderr)

	// here the default context logger is set, but initStandaloneModeLogger is only called on sdk.serve, not when I'm developing connectors
	zerolog.DefaultContextLogger = &logger 
	return nil
}

It might be confusing for future connector developers to not see any logs when calling sdk.Logger(ctx).Info(), as it was confusing for me.

Solutions to this problem that I can think of:

  1. make initStandaloneModeLogger public, and allow sdk consumers to initialize the logger
  2. add an init call in logger.go that sets up zerolog.DefaultContextLogger
  3. add an additional InitLogger function that does the same as initStandaloneModeLogger, but meant for testing purposes.

I have to add also that this is not that important for me anymore, as I know now why I did not see logs on my tests. I can just set the DefaultContextLogger myself on my test code.

@alarbada alarbada added feature New feature or request triage Needs to be triaged labels Feb 7, 2024
@github-project-automation github-project-automation bot moved this to Triage in Conduit Main Feb 7, 2024
@simonl2002 simonl2002 added wontfix This will not be worked on and removed triage Needs to be triaged labels Feb 12, 2024
@simonl2002
Copy link
Member

We won't implement this specific change but we will update the documentation.

@github-project-automation github-project-automation bot moved this from Triage to Done in Conduit Main Feb 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request wontfix This will not be worked on
Projects
Archived in project
Development

No branches or pull requests

2 participants