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

Support normal request logging #49

Open
dinvlad opened this issue Sep 26, 2020 · 10 comments
Open

Support normal request logging #49

dinvlad opened this issue Sep 26, 2020 · 10 comments
Labels
enhancement New feature or request P3 Low Priority

Comments

@dinvlad
Copy link

dinvlad commented Sep 26, 2020

Hi Team,

Would it be possible to enable optional logging of all requests? For HTTP functions, it'd be helpful to see response status and some basic metrics like request latency, while developing locally.

Thanks

@grant
Copy link
Contributor

grant commented Oct 7, 2020

Sorry, can you be more specific? What is an expected command/feature that doesn't work right now?

Within your function, you can log on all requests as normal.

log.Print("foobar")

If you're running a compiled Go binary, you can just use normal bash tools like time ./my-function to see request latency.

@dinvlad
Copy link
Author

dinvlad commented Oct 7, 2020

Hi @grant - I'm referring to automatic logging of all requests by this library, as opposed to only the errors. For sure, we could add logging in our own code, but I was curious if this could be done out-of-the-box, similarly to how for example each request gets logged in Nginx or similar proxies. Does that make sense? Thanks

@grant
Copy link
Contributor

grant commented Oct 7, 2020

Oh, I don't think we have that feature, logging all requests, in the frameworks.

We could add a sample that does that.

We want to be careful of not logging PII, as it's very easy to create a vulnerable system if you just all requests. Was just talking to @grayside about this.

@grayside
Copy link

grayside commented Oct 7, 2020

Request logging is already handled when the framework is deployed to GCF, sounds like this ask is specifically to add some minimal request logging for local development. I think that makes sense.

Something along the lines of a simplified nginx access log, or replicating the HTTPRequest object from the GCF request log.

@dinvlad
Copy link
Author

dinvlad commented Oct 7, 2020

Makes sense, thanks for clarifying it! I was thinking to add only general logging however, like the response status and possibly request latency, possibly only for local development indeed. Exactly how it's done in Nginx!

@dinvlad
Copy link
Author

dinvlad commented Oct 7, 2020

Basically, something to indicate that a request was done and its status/run time.

@grant
Copy link
Contributor

grant commented Oct 7, 2020

A pull request / forked example would mean a lot in terms of understanding the change. If this feature isn't added to the framework, you can always deploy your function to Cloud Run.

@tbpg
Copy link

tbpg commented Oct 12, 2020

If we exposed the middleware that takes the HTTP request and calls the user's function, users could register the handlers however they want. Part of that includes adding additional logging as desired.

There are a lot of different ways to do logging. I don't think we should pick a way to do it in this project.

Note: this also came up with #25.

@grayside
Copy link

I think exposing an ability to add middlewares is a good way to facilitate this use case, leveraging the existing ecosystem of middlewares examples and libraries, and inviting developers to create reusable utilities.

The challenge might be in helping differentiate by environment: we don't want users to add request logging in GCF, do they'd want to make this change in main() instead of in their function file.

@dinvlad
Copy link
Author

dinvlad commented Oct 12, 2020

Agreed, I think it'd be nice to expose a common way to add middlewares in main(), which would make the framework extensible to cover any needs of development for any runtime.

@josephlewis42 josephlewis42 added enhancement New feature or request P3 Low Priority labels Mar 1, 2023
@kenneth-rosario kenneth-rosario added the blunderbuss: assign Instruct blunderbuss to assign someone label Apr 13, 2023
@blunderbuss-gcf blunderbuss-gcf bot removed the blunderbuss: assign Instruct blunderbuss to assign someone label Apr 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request P3 Low Priority
Projects
None yet
Development

No branches or pull requests

7 participants