nestjs-slack
nestjs-slack copied to clipboard
bjerkio
Feat remove google logger
Sorry for being unclear. I don't think we should remove it entirely, but make it a peer dependency.
What do you think about it? There are users who are pushing this to the Google Cloud logger, and picking up with tools liks gcl-slack. But again, having used this setup for many years, I'm no longer sure that is a good approach. Very curious what you think we should do.
@simenandre I think a good design approach for this would be for the application to instantiate the google logger and set nestjs to use that logger. by doing that, this module will use the google logger. this module shouldn't need google libraries as a dependency or a peer dependency IMO.. separate concerns
but I understand not wanting to make this change... potentially a major version change in this case if you want to proceed with this approach
It's fine for me that we break stuff, as long as it's easy to migrate 👍
Google Cloud Logger isn't really used for logging in our case, it's used to push a Slack object to a log. We could use the generic NestJS logger for this, or even console.log as long as we are sending JSON the way Google Cloud logger wants.
The design behind using Google Cloud is to use the log as a transport for Slack, removing requests for the application layer. Another application picks up this log message and sends it to Slack, i.e. a Cloud Function or otherwise. Could be done the same way in AWS, Azure and Kubernetes, etc. However, I'm not sure about this design and if it's worth maintaining.
Ah I see. That makes sense. Well I'll leave the decision up to you.. feel free to close the MR if you want to not proceed.
It probably can be configured to work with @google logger as a peerDependenecy but I'm afraid my @nestjs module development skills are not quite there to make the module's logger injection be optional