Melkeydev / go-blueprint

Go-blueprint allows users to spin up a quick Go project using a popular framework
https://docs.go-blueprint.dev/
MIT License
4.5k stars 268 forks source link

[Feature Request] Logging support #182

Open spa5k opened 9 months ago

spa5k commented 9 months ago

Tell us about your feature request

Something like global setup of loggers, zap/slog/zerolog etc, and optimal way of importing the logger in various places

Disclaimer

Ujstor commented 8 months ago

We believe that incorporating Slog or the standard logging library from Go would be the most suitable approach.

mcheviron commented 8 months ago

We believe that incorporating Slog or the standard logging library from Go would be the most suitable approach.

Will you add it or should someone make a pull request? Also for whoever does it, I suggest creating an abstraction over LogAttrs instead of using the slog API. Otherwise each log will alloc on the heap, unlike Zap that uses this strategy by default

Ujstor commented 8 months ago

This is open for anyone willing to step forward and address this enhancement. Just let me know, and I can assign it

briancbarrow commented 8 months ago

Also for whoever does it, I suggest creating an abstraction over LogAttrs instead of using the slog API. Otherwise each log will alloc on the heap, unlike Zap that uses this strategy by default

@mcheviron Do you have any suggested reading about this before someone takes this on?

mcheviron commented 8 months ago

Also for whoever does it, I suggest creating an abstraction over LogAttrs instead of using the slog API. Otherwise each log will alloc on the heap, unlike Zap that uses this strategy by default

@mcheviron Do you have any suggested reading about this before someone takes this on?

https://pkg.go.dev/log/slog@go1.21.3#LogAttrs

The interface would be simple. A simple logger struct that exposes the same API but has a private context inside of it to enable the LogAttrs method. Then this would allow the caller to just send messages to the API for .Info, .Debug etc and the user won't feel a difference but the allocations will remain in the stack. This is how Zap does it internally. The standard slog API uses any which is a heap allocation. You can go run -gcflags "-m" and observe it

As for implementing it. How would you like implementing it? Should this logger be put in a separate file in the cmd dir or how would it work for your project? I have the code. I created it for work but i'd like to know how best is it to contribute it to your project

P.S: https://www.reddit.com/r/golang/comments/19882h2/slog_why_do_my_attrs_escape_to_the_heap/?utm_source=share&utm_medium=web2x&context=3 Not quality content but we discussed it there before

spa5k commented 8 months ago

Rather than this, what if we use slog, then implement its handlers in various loggers, like zap etc.

package main

import (
    "log/slog"

    "go.uber.org/zap"
    "go.uber.org/zap/exp/zapslog"
)

func main() {
    zapL := zap.Must(zap.NewProduction())

    defer zapL.Sync()

    logger := slog.New(zapslog.NewHandler(zapL.Core(), nil))

    logger.Info(
        "incoming request",
        slog.String("method", "GET"),
        slog.String("path", "/api/user"),
        slog.Int("status", 200),
    )
}
mcheviron commented 8 months ago

I don't see the value of adding Zap since the original commenter wanted to use slog to keep things in the stdlib. If all you need is to pass the Zap core to slog then why not just stick with zap? That sounds like over engineering something, at least for me.

If they agree, we can handle logging like web frameworks, expose the stdlib only option as the suggested but offer Zap and friends as options.

Ujstor commented 8 months ago

@mcheviron, if you agree, I would like to assign this to you. The solution needs to be simple and without overengineering, working out of the box and being incorporated into the blueprint (users do not have a choice in the CLI).

You can put logging.go in internal/utils. I am not familiar with common practices and we are open for any suggestions.

Maybe you can create a test repo from go-blueprint to see how everything works together.

You already have the code, you know what you're doing.

mcheviron commented 8 months ago

@mcheviron, if you agree, I would like to assign this to you. The solution needs to be simple and without overengineering, working out of the box and being incorporated into the blueprint (users do not have a choice in the CLI).

You can put logging.go in internal/utils. I am not familiar with common practices and we are open for any suggestions.

Maybe you can create a test repo from go-blueprint to see how everything works together.

You already have the code, you know what you're doing.

Hi thanks. Please assign it to me and I shall get to it in the weekend. Is there a deadline? internal/utils is a sensible approach since it's supposed to be internal and a util

I'll do a fork and I'll test in my environment before I pul ofc

Ujstor commented 8 months ago

I was thinking that you implement it first in a project that is created with a blueprint, then we can finalize the structure and functionality. After that, you can simply add changes to existing templates and new logic for creating the utils folder and logging.go. That was just an idea, but you can implement it directly in the project.

There is no deadline, take your time.

mcheviron commented 8 months ago

I was thinking that you implement it first in a project that is created with a blueprint, then we can finalize the structure and functionality. After that, you can simply add changes to existing templates and new logic for creating the utils folder and logging.go. That was just an idea, but you can implement it directly in the project.

https://github.com/mcheviron/logger-test

Take a look and tell me if this is what you wanted. I tweaked my original implementation by creating a config that the user can use to customise the output (json or text for example). I also added some helpful defaults to smoothen the rough edges of slog. For example making the logger log only the base file if the user asked to add source instead of the absolute path. I also added how they can automatically take away the timestamp in case they run on a cloud. AWS CloudWatch and the respective GCP and Azure options inject timestamps by default. This one is commented, they can uncomment it if they need it.

Ujstor commented 8 months ago

This looks nice, exactly what I was thinking about. What do you think about adding a middleware method? Chi and Echo are using built-in middleware. Can we unify that and use it for all frameworks?

mcheviron commented 8 months ago

This looks nice, exactly what I was thinking about. What do you think about adding a middleware method? Chi and Echo are using built-in middleware. Can we unify that and use it for all frameworks?

we can but i don't suggest it. I don't know about chi (never used it) but echo has a way to inject your favourite logger into the framework's logger if you want. so my advice would be making the logger's template present for all users no matter their favourite framework and if they want to plug it, they do it. because echo already has a logging middleware that can be used for logging requests, but the logger i created is for tracing purposes mainly.

if a user chose to use a framework, we should get least into their way and let them figure their stuff out, since they already should be familiar with its ecosystem.

what i suggest is just provide the logger. afterwards i was going to make a pull request anyway to provide minimal code (less than 100 LOC) that kickstarts the 1.22 router and make it easy to configure. if you go on my repos i have a repo called sdk that has that code (some of it at least). i use it for work as a minimal http stdlib only sdk that makes injecting middleware, logging etc easy without third party stuff. but that's for later.

if you're okay with that, just logger. i'll look into the template stuff in the create command and figure out how to inject it in a fork and then when it's okay i'll do a pull. cheers

Ujstor commented 8 months ago

I agree, the best way is not to overcomplicate things. When you are ready, open a PR, and I will review it.

Ujstor commented 8 months ago

@mcheviron We have PR #187. I think the best way is to wait until it is merged because it will introduce changes that affect you. Afterward, it would also be simple to implement the logger

Ujstor commented 8 months ago

@mcheviron PR has been merged, you can open yours when you are ready.

mcheviron commented 8 months ago

@Ujstor thank you. i'll get to it in the weekend

mcheviron commented 8 months ago

@Ujstor

I was thinking of putting the template file here:

Then, I'll add the Go embedding here and expose a LoggerTemplate:

Next, I'll add a case in the CreateFileWithInjection function and call it somewhere in the CreateMainFile function.

After this block of code:


    err = p.CreateFileWithInjection(internalServerPath, projectPath, "server.go", "server")
    if err != nil {
        log.Printf("Error injecting server.go file: %v", err)
        cobra.CheckErr(err)
        return err
    }

(i'm adding these comments as confirmation and for me to not forget because i'm a dummy)

Anyways, if ok, cool it'll be done in a couple of minute, not? tell me what you want different. thanks!

Ujstor commented 8 months ago

@mcheviron Looks good to me.

Ujstor commented 8 months ago

@mcheviron Join Discord you can find me there https://discord.gg/Kx8Zz46aQS

H0llyW00dzZ commented 7 months ago

I have implemented it a custom logger for logging support.

Example of how it works:

With ENV UNCENSORED set to False (for Production/Server Less)

$ go run cmd/api/main.go
2024/03/31 14:44:15 [H0llyW00dzZ] [INFO] Starting server on :****

 ┌───────────────────────────────────────────────────┐
 │                    H0llyW00dzZ                    │
 │                   Fiber v2.52.4                   │
 │               http://127.0.0.1:8080               │
 │       (bound on host 0.0.0.0 and port 8080)       │
 │                                                   │
 │ Handlers ............ 534 Processes ........... 1 │
 │ Prefork ....... Disabled  PID .............. 6860 │
 └───────────────────────────────────────────────────┘

2024/03/31 14:44:22 [H0llyW00dzZ] [INFO] Shutting down server... reason: interrupt
2024/03/31 14:44:23 [H0llyW00dzZ] [INFO] Disconnected from database: *********
2024/03/31 14:44:23 [H0llyW00dzZ] [INFO] Database connection closed.
2024/03/31 14:44:27 [H0llyW00dzZ] [INFO] Server exiting

[!NOTE] With ENV UNCENSORED set to False (for Production/Server Less), it will CENSOR any object formatting object in Golang (e.g., %v, %s, etc).

With ENV UNCENSORED set to True


$ go run cmd/api/main.go
2024/03/31 14:44:15 [H0llyW00dzZ] [INFO] Starting server on :8080

 ┌───────────────────────────────────────────────────┐
 │                    H0llyW00dzZ                    │
 │                   Fiber v2.52.4                   │
 │               http://127.0.0.1:8080               │
 │       (bound on host 0.0.0.0 and port 8080)       │
 │                                                   │
 │ Handlers ............ 534 Processes ........... 1 │
 │ Prefork ....... Disabled  PID .............. 6860 │
 └───────────────────────────────────────────────────┘

2024/03/31 14:44:22 [H0llyW00dzZ] [INFO] Shutting down server... reason: interrupt
2024/03/31 14:44:23 [H0llyW00dzZ] [INFO] Disconnected from database: defaultdb
2024/03/31 14:44:23 [H0llyW00dzZ] [INFO] Database connection closed.
2024/03/31 14:44:27 [H0llyW00dzZ] [INFO] Server exiting
H0llyW00dzZ commented 7 months ago

Also note that the custom logger I have implemented is legit made from scratch and is currently only compatible with the standard library logger.

Ujstor commented 7 months ago

This is a work in progress, and it would take care of proper logging and error handling. Here is draft https://github.com/mcheviron/test

H0llyW00dzZ commented 7 months ago

This is a work in progress, and it would take care of proper logging and error handling. Here is draft https://github.com/mcheviron/test

By the way, it's easy to implement logging and error handling for the Fiber framework, especially for custom loggers.

Example:

// LogUserActivity logs a user activity message along with the client IP and User-Agent.
func LogUserActivity(c *fiber.Ctx, activity string) {
    clientIP := c.IP()
    userAgent := c.Get("User-Agent")
    // Note: `LogVisitorf` is the handler for the example logger using the standard library
    LogVisitorf("Activity: %s - IP: %s, User-Agent: %s", activity, clientIP, userAgent)
}

Then, call it in the handler:

// Log the user activity
Logger.LogUserActivity(c, "viewed firewall IPs")

The output will look like this:

2024/04/04 16:19:07 [H0llyW00dzZ Project] [VISITOR] Activity: viewed firewall IPs - IP: 127.0.0.1, User-Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:15.0) Gecko/20100101 Firefox/15.0.1

The provided code snippet demonstrates a simple way to implement logging for user activities in the Fiber framework. It includes a LogUserActivity function that logs the user's activity, IP address, and User-Agent string using a custom logger (LogVisitorf). This function can be called within the handler to log relevant user activities.

The example output shows how the logged information would appear, including the timestamp, a custom prefix ([H0llyW00dzZ Project] [VISITOR]), and the activity details.