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

docs(asgi-tutorial): include info on setting up logging for debugging #2223

Merged
merged 8 commits into from May 7, 2024

Conversation

MRLab12
Copy link
Contributor

@MRLab12 MRLab12 commented Apr 3, 2024

Summary of Changes

This PR introduces a new section to the ASGI tutorial in the Falcon documentation, aimed at providing developers with clear instructions on setting up and utilizing logging for debugging ASGI applications. The added content outlines the steps to configure basic logging using Python's built-in logging module, offering examples of how to log informational, warning, and error messages effectively.

Related Issues

#2098

Pull Request Checklist

This is just a reminder about the most common mistakes. Please make sure that you tick all appropriate boxes. But please read our contribution guide at least once; it will save you a few review cycles!

If an item doesn't apply to your pull request, check it anyway to make it apparent that there's nothing to do.

  • Applied changes to both WSGI and ASGI code paths and interfaces (where applicable).
  • Added tests for changed code.
  • Prefixed code comments with GitHub nick and an appropriate prefix.
  • Coding style is consistent with the rest of the framework.
  • Updated documentation for changed code.
    • Added docstrings for any new classes, functions, or modules.
    • Updated docstrings for any modifications to existing code.
    • Updated both WSGI and ASGI docs (where applicable).
    • Added references to new classes, functions, or modules to the relevant RST file under docs/.
    • Updated all relevant supporting documentation files under docs/.
    • A copyright notice is included at the top of any new modules (using your own name or the name of your organization).
    • Changed/added classes/methods/functions have appropriate versionadded, versionchanged, or deprecated directives.
  • Changes (and possible deprecations) have towncrier news fragments under docs/_newsfragments/, with the file name format {issue_number}.{fragment_type}.rst. (Run towncrier --draft to ensure it renders correctly.)

If you have any questions to any of the points above, just submit and ask! This checklist is here to help you, not to deter you from contributing!

PR template inspired by the attrs project.

@vytas7 vytas7 changed the title Update tutorial-asgi to include info on setting up logging for debugging docs(asgi-tutorial): include info on setting up logging for debugging Apr 3, 2024
Copy link

codecov bot commented Apr 3, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (c4a5f32) to head (83f7b41).

Additional details and impacted files
@@            Coverage Diff            @@
##            master     #2223   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           63        63           
  Lines         6931      6931           
  Branches      1258      1258           
=========================================
  Hits          6931      6931           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@CaselIT CaselIT requested a review from vytas7 April 5, 2024 19:39
Copy link
Member

@CaselIT CaselIT left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the lines should in the rst go to a new line at about 80-100 chars.

Other than that it seems a good addition, thanks!

logger.exception('An error occurred: %s', str(e))


For more sophisticated logging setups (e.g., different log levels or formats for development and production), you can configure multiple handlers and formatters, as described in the Python logging documentation.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we may want to link to the external logging docs: https://docs.python.org/3/howto/logging.html#logging-basic-tutorial

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, good idea.

Debugging ASGI Applications
---------------------------

While developing and testing ASGI applications, understanding how to configure and utilize logging can be helpful, especially when you encounter unexpected issues or behaviors.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could add "This section applies also when using WSGI application"

@MRLab12 MRLab12 marked this pull request as ready for review April 11, 2024 00:44

While developing and testing ASGI applications, understanding how to configure and utilize logging can be helpful, especially when you encounter unexpected issues or behaviors.

By default, Falcon does not set up logging for you, but Python's built-in `logging` module provides a flexible framework for emitting and capturing log messages. Here's how you can set up basic logging in your ASGI Falcon application:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIRC, we have intersphinx connected to the stdlib's docs, so you probably can even link to them:

:mod:`logging`

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah had not seen there was an intersphinx mapping. Doing this change.

Copy link
Member

@vytas7 vytas7 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this improvement, it looks good for the most of it!

I've left some suggestions inline, and as pointed out by @CaselIT, we should shorten RST lines to a reasonable length around 80 characters.

logger.warning('Warning message')
logger.error('Error message')

It's especially useful to log exceptions and error messages that can help diagnose issues:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a good intro on logging, but IMHO the reader of this second tutorial is already expected to be an experienced Python developer, so maybe we could shorten this generic section a bit, and tell more about Falcon's default Exception handler?

In addition to rendering an HTTP 500 response, it does log a message to the falcon logger. Maybe we could even show how this looks like?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah totally makes sense. Is something like this what you have in mind?

import falcon
import logging

logging.basicConfig(level=logging.INFO)

class ErrorResource:
    def on_get(self, req, resp):
        raise Exception("Something went wrong!")

app = falcon.App()
app.add_route('/error', ErrorResource())


class ErrorResource:
def on_get(self, req, resp):
raise Exception("Something went wrong!")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we are using single quotes throughout the codebase

Copy link
Member

@vytas7 vytas7 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I took the liberty of changing your Exception line to use single quotes, looks good otherwise now, thanks for this improvement!

Another thought is that we probably want to move this section somewhere closer to the start, because it is likely that the reader might encounter errors earlier 🙂 But we can follow up on this later. LGTM as is for now.

@vytas7 vytas7 requested a review from CaselIT May 6, 2024 20:12
@vytas7
Copy link
Member

vytas7 commented May 6, 2024

Oh, it seems that we have some CI fighting to do that is unrelated to the proposed changes 😬 I'll take a look at these tomorrow.

Edit: hopefully done with these.

Copy link
Member

@CaselIT CaselIT left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good to me

@vytas7 vytas7 merged commit 9b27c71 into falconry:master May 7, 2024
37 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants