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

Customize smtp traffic log format #136

Closed
Dennis-Zhang-SH opened this issue Jul 24, 2023 · 13 comments · Fixed by #141
Closed

Customize smtp traffic log format #136

Dennis-Zhang-SH opened this issue Jul 24, 2023 · 13 comments · Fixed by #141
Labels
enhancement New feature or request WIP Work is in progress

Comments

@Dennis-Zhang-SH
Copy link

Is your feature request related to a problem? Please describe.

No response

Describe the solution you'd like

pass a format string and logging with the format.

Describe alternatives you've considered

fork and chang the log message in forked repo

Additional context

No response

@Dennis-Zhang-SH Dennis-Zhang-SH added the enhancement New feature or request label Jul 24, 2023
@wneessen
Copy link
Owner

Hi @Dennis-Zhang-SH, thanks for opening this issue. Can you elaborate a bit more what you are actually requesting? The standard logger we ship with go-mail does support formatted logging already - in fact it's even required by the interface that Debugf, Infof, Warnf and Errorf are satisfied.

@Dennis-Zhang-SH
Copy link
Author

Hi @Dennis-Zhang-SH, thanks for opening this issue. Can you elaborate a bit more what you are actually requesting? The standard logger we ship with go-mail does support formatted logging already - in fact it's even required by the interface that Debugf, Infof, Warnf and Errorf are satisfied.

Hi I mean the hard coded "C <-- S:" and "C --> S:", it would be nice to have self defined format, say "Sent: msg" and "Reply (code): msg"

@wneessen
Copy link
Owner

Thanks for clarifying. I'll see what would be the best approach for this

@wneessen
Copy link
Owner

So I see two possible options here:

  • We can add a new option to the Client for you to provide the In- and Out logging strings. But that would require adding new fields to the Client and smtp.Client structs, which might be not favourable.
  • Since this is very much an edge case, we can check the ENV variables for the in- and out strings directly in the method in the smtp client and if set, we could override the defaults. This would be the most favourable option but might not work in every edge case, neither.

What are your thoughts?

@Dennis-Zhang-SH
Copy link
Author

So I see two possible options here:

  • We can add a new option to the Client for you to provide the In- and Out logging strings. But that would require adding new fields to the Client and smtp.Client structs, which might be not favourable.
  • Since this is very much an edge case, we can check the ENV variables for the in- and out strings directly in the method in the smtp client and if set, we could override the defaults. This would be the most favourable option but might not work in every edge case, neither.

What are your thoughts?

I think maybe providing the raw logs would be better? people can manipulate the logs by themselves.

@wneessen
Copy link
Owner

That's an option, but the communication is two-way. How would we differentiate between message sent by the client or by the server without making the logging interface much more complex?

@Dennis-Zhang-SH
Copy link
Author

That's an option, but the communication is two-way. How would we differentiate between message sent by the client or by the server without making the logging interface much more complex?

something like this?

type Logs struct {
	Recv []RecvLog
	Sent []string
}

type RecvLog struct {
	Code int
	Msg  string
}

func (l *Logs) Debug() {
	for _, m := l.Recv {
		logger.Debug("s ---> c", m)
	}
}

then you can setup a new Debug func to overwrite the default debug func.

@wneessen wneessen added the WIP Work is in progress label Aug 1, 2023
wneessen added a commit that referenced this issue Aug 2, 2023
The logging system in the smtp.go and log package has been refactored. A new custom log type `Log` was introduced, that includes the message direction, format and arguments. The `Logger` interface and the `Stdlog` implementation were modified to accept this new type. This change provides a clearer understanding of message direction inside logs, allowing for easier debugging and reduced complexity. This change does not affect features or disrupt user functionality. Additionally, it allows for custom implementations of the log.Logger interface to without being forced to use the C --> S/C <-- S direction logging.
@wneessen
Copy link
Owner

wneessen commented Aug 2, 2023

I've proposed a change to the log.Logger interface as well as to the Stdlog logger. It breaks the old interface but allows the logging interface to be much more flexible and should allow you to use your own formatting. Feedback is welcome.

If we accept this change, we need to mark it as breaking, even though I don't think that many users are actually using their own logging interface in their implementations.

@Dennis-Zhang-SH
Copy link
Author

the default log format should be "C ---> S:" and it should be able to overwrite, but now it's like "C ---> S: myformat", if you don't mind, I can work on this later and try to find a better way to solve this, and probably won't be a breanking change,

@wneessen
Copy link
Owner

wneessen commented Aug 2, 2023

The default format for the StdLoggerwas not changed. It was C --> S: <format> <message> before this change. It was only moved from the smtp package to the log package. For you to not use the C --> S syntax, all you need to do is implement your own Debugf() function that satisfies the log.Logger interface and then you can completely skip the C --> S part or format it in the way you want.

@Dennis-Zhang-SH
Copy link
Author

The default format for the StdLoggerwas not changed. It was C --> S: <format> <message> before this change. It was only moved from the smtp package to the log package. For you to not use the C --> S syntax, all you need to do is implement your own Debugf() function that satisfies the log.Logger interface and then you can completely skip the C --> S part or format it in the way you want.

Oh okay, that should fix it, thanks for your work and appologize for my late reply.

wneessen added a commit that referenced this issue Aug 8, 2023
In the updated version of msgwriter.go, an additional error handling process has been included. If an error is detected when creating a new part in the message writer, this error is stored and prevents executing the writeBody function. This fixes nil pointer dereference in `mw.writeBody` if an error occured previously
@wneessen
Copy link
Owner

Are you ok with the proposal? In that case i would merge it to main then.

@Dennis-Zhang-SH
Copy link
Author

Are you ok with the proposal? In that case i would merge it to main then.

Yes I am fine with it, you can close this issue after merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request WIP Work is in progress
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants