Skip to content

docs: Add basic logging documentation #926

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

Merged
merged 22 commits into from
Jul 8, 2025
Merged

Conversation

dan-fernandes
Copy link
Contributor

No description provided.

Copy link

codecov bot commented Apr 22, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 94.47%. Comparing base (24a3cfd) to head (481fb2b).
Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #926   +/-   ##
=======================================
  Coverage   94.47%   94.47%           
=======================================
  Files          41       41           
  Lines        2551     2551           
=======================================
  Hits         2410     2410           
  Misses        141      141           

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@dan-fernandes dan-fernandes changed the title Add basic logging documentation docs: Add basic logging documentation Apr 22, 2025
Copy link
Contributor

@DominicOram DominicOram left a comment

Choose a reason for hiding this comment

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

Minor comments. Some additional questions:

  • So this isn't enabled by default? Why not just make a bluesky stream in graylog and log into that by default? I see no reason anyone would not want it on?
  • If we do just enable it as default I think the info about fluentd is too much detail. Instead you just need to say where it's going by default and how to change it if you need to
  • A link to our graylog instance would be good, even better if it's directly to the default stream
  • What fields are added by default? Can the user change fields if they want to? This is probably a more "Advanced" feature

@dan-fernandes
Copy link
Contributor Author

dan-fernandes commented May 1, 2025

@DominicOram

So this isn't enabled by default?

If it's enabled by default you would log to the Graylog instance by just checking out / spinning up the repo while on a Diamond computer. Would a better compromise be having Graylog enabled by default, but to localhost?

The rest I agree with and will implement before requesting another review

@DominicOram
Copy link
Contributor

Hmm... Does blueAPI have no general way of knowing if you're in dev or prod? Is there any plan for this? I appreciate it's outside the scope of the issue so happy for it not to be on as default for now

@DiamondJoseph
Copy link
Contributor

Hmm... Does blueAPI have no general way of knowing if you're in dev or prod? Is there any plan for this? I appreciate it's outside the scope of the issue so happy for it not to be on as default for now

I don't think there's any plans for that- development can mean so many things and a single setting can't encapsulate them all:
Does dev mean development of the blueapi service or of plans and devices?
Is any deployment in kubernetes "production"?

What does dev mean in terms of behaviour?

  • Doesn't connect/connects mock mode/connects to particular [sim?] devices?
  • Doesn't connect to a message bus/connects to a local/specific message bus
  • Doesn't send to graylog? Sends to local development graylog? Logs at DEBUG?
  • Doesn't use an identity provider/uses a mock/mocks the impl?
  • Doesn't talk to numTracker/talks to a dummy instance/mocks the impl?

All of these are things you sometimes need to do while developing blueapi, and half of them are things you might need to do while developing plans.

I think the config should just expose as much as is reasonable, and provide convenient defaults that work without any external infrastructure, both when starting the service locally/in a container and when installing the Helm chart to a fresh cluster.

Copy link
Contributor

@callumforrester callumforrester 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, a few suggestions

@DiamondJoseph DiamondJoseph requested a review from a team as a code owner June 13, 2025 13:56

# BlueAPI Cofiguration

:::{seealso}
Copy link
Contributor

Choose a reason for hiding this comment

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

Which version of markdown supports the :::{seealso} tags?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's a sphinx directive.

This builds into BlueAPI's GitHub Pages entry.

@DominicOram DominicOram dismissed their stale review June 17, 2025 13:10

Happy to park discussions of enabling by default in favour of getting documentation in.

@dan-fernandes dan-fernandes requested a review from tpoliaw July 1, 2025 07:47
Copy link
Contributor

@DiamondJoseph DiamondJoseph left a comment

Choose a reason for hiding this comment

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

Approving with potential rewording because if I had my way I would rewrite all of the docs myself and I don't want to do that


# Configure Logging

By default BlueAPI will log to stdout at the [INFO level](https://docs.python.org/3/library/logging.html#logging-levels), but can be reconfigured to log at any level, and to output to Graylog.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
By default BlueAPI will log to stdout at the [INFO level](https://docs.python.org/3/library/logging.html#logging-levels), but can be reconfigured to log at any level, and to output to Graylog.
By default BlueAPI will log to stdout at the [INFO level](https://docs.python.org/3/library/logging.html#logging-levels), but can be configured to log at any level, and to output simultaneously at the same level to [Graylog](https://graylog.org).

Comment on lines +5 to +7

When logging to [Graylog](https://graylog.org) is enabled, BlueAPI will also continue to log to stdout.

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
When logging to [Graylog](https://graylog.org) is enabled, BlueAPI will also continue to log to stdout.

graylog:
enabled: True
host: "graylog-log-target.diamond.ac.uk"
port: 12232
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
port: 12232
port: 12231

We should adjust the defaults to hit the right graylog too

Comment on lines +27 to +29
To instrument to a custom module in BlueAPI, instantiate a logger from the [standard library logging package, then use any of its log methods](https://docs.python.org/3/library/logging.html#logger-objects).

BlueAPI is configured to handle logging from any python code it executes.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
To instrument to a custom module in BlueAPI, instantiate a logger from the [standard library logging package, then use any of its log methods](https://docs.python.org/3/library/logging.html#logger-objects).
BlueAPI is configured to handle logging from any python code it executes.
Logs from plans run by BlueAPI will use inherit the configured logging, simply instantiate a logger from the [standard library logging package, then use any of its log methods](https://docs.python.org/3/library/logging.html#logger-objects).

Copy link
Contributor

Choose a reason for hiding this comment

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

Does this mean plans are able to reconfigure the central logging system as well?

Comment on lines +32 to +34
import logging
logger = logging.getLogger(__name__)
logger.info("FOO")
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
import logging
logger = logging.getLogger(__name__)
logger.info("FOO")
import logging
logger = logging.getLogger(__name__)
def my_plan() -> MsgGenerator:
logger.info("Running my_plan")
logger.debug("Loaded module with my_plan")

Comment on lines +39 to +43
Services hosted on the DLS clusters automatically have their stdout forwarded to Graylog via a service called fluentd. Due to this, BlueAPI services hosted on the cluster will always log to Graylog.

When BlueAPI's native Graylog support is enabled it forwards structured data rather than plaintext.

When BlueAPI's native Graylog is enabled fluentd will be automatically disabled to avoid log duplication.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Services hosted on the DLS clusters automatically have their stdout forwarded to Graylog via a service called fluentd. Due to this, BlueAPI services hosted on the cluster will always log to Graylog.
When BlueAPI's native Graylog support is enabled it forwards structured data rather than plaintext.
When BlueAPI's native Graylog is enabled fluentd will be automatically disabled to avoid log duplication.
Services hosted on the DLS clusters automatically have their stdout forwarded to Graylog via a service called fluentd. Therefore BlueAPI hosted on the cluster will always have logs available from Graylog, however BlueAPI's native Graylog forwards structured data rather than plaintext.
When native Graylog is enabled fluentd will be automatically disabled to avoid log duplication.

@dan-fernandes dan-fernandes merged commit bec4e11 into main Jul 8, 2025
17 checks passed
@dan-fernandes dan-fernandes deleted the add-logging-documentation branch July 8, 2025 07:34
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.

5 participants