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

Logger backend #829

Merged
merged 7 commits into from
Mar 16, 2023
Merged

Logger backend #829

merged 7 commits into from
Mar 16, 2023

Conversation

jeffkreeftmeijer
Copy link
Member

@jeffkreeftmeijer jeffkreeftmeijer commented Mar 2, 2023

With the new logger backend, users can redirect their logs to AppSignal by attaching the backend in their application.ex files:

Logger.add_backend({Appsignal.Logger.Backend, [group: "phoenix"]})

Closes #800.

Previously, Appsignal.Logger only exposed the named functions (like
debug/3). Since we need the log/4 function to provide a backend, this
patch eposes it.
Appsignal.Logger.Backend is attached to Elixir's logger to send logs to
AppSignal. Use Logger.add_backend/1 to set it up:

    Logger.add_backend({Appsignal.Logger.Backend, "phoenix"})

The second argument in the tuple is the log group name, which could be
"phoenix" for a Phoenix application, for example.
> Add Logger backend to redirect Elixir logs to AppSignal.
@unflxw
Copy link
Contributor

unflxw commented Mar 2, 2023

With the new logger backend, users can redirect their logs to AppSignal by attaching the backend in their application.ex files:

Logger.add_backend({Appsignal.Logger.Backend, "phoenix”})

It should also be possible to configure it in the application environment, like in config.exs or runtime.exs, right? Quoting the docs for add_backend/2:

config :logger, :backends, [{Appsignal.Logger.Backend, "phoenix"}]

I'm asking specifically about this because I get the impression that this would be the preferred usage pattern, and I think we should prioritise this in documentation over add_backend/2 (please let me know if you disagree!)

Copy link
Contributor

@unflxw unflxw left a comment

Choose a reason for hiding this comment

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

What happens in a scenario where AppSignal is not active? Say, if this backend is configured in the general config, but AppSignal is only active in certain environments. I think the call to Appsignal.Nif might break, but I don't really know enough about the Nif setup to be sure.

lib/appsignal/logger/backend.ex Outdated Show resolved Hide resolved
lib/appsignal/logger/backend.ex Outdated Show resolved Hide resolved
lib/appsignal/logger/backend.ex Outdated Show resolved Hide resolved
Co-authored-by: Noemi <45180344+unflxw@users.noreply.github.com>
@jeffkreeftmeijer
Copy link
Member Author

With the new logger backend, users can redirect their logs to AppSignal by attaching the backend in their application.ex files:

Logger.add_backend({Appsignal.Logger.Backend, "phoenix”})

It should also be possible to configure it in the application environment, like in config.exs or runtime.exs, right? Quoting the docs for add_backend/2:

config :logger, :backends, [{Appsignal.Logger.Backend, "phoenix"}]

I'm asking specifically about this because I get the impression that this would be the preferred usage pattern, and I think we should prioritise this in documentation over add_backend/2 (please let me know if you disagree!)

Yes, and agreed.

@backlog-helper

This comment has been minimized.

@jeffkreeftmeijer
Copy link
Member Author

What happens in a scenario where AppSignal is not active? Say, if this backend is configured in the general config, but AppSignal is only active in certain environments. I think the call to Appsignal.Nif might break, but I don't really know enough about the Nif setup to be sure.

If AppSignal is not active, the log/2 function is a noop, so that’s accounted for. :)

Instead of taking a string as the log group, take a keyword list with a
single key, for possible extension later.
@backlog-helper

This comment has been minimized.

Deprecated, but still used in older versions of Elixir, warnings named
:warn previously broke the logger. This patch accepts them as warnings.
Although this shouldn't happen, not finding a severity in the list
caused Appsignal.Logger.Backend to crash. This patch adds a rescue
clause to turn everything that doesn't match the list to a severity of
3 (info).
@backlog-helper

This comment has been minimized.

1 similar comment
@backlog-helper
Copy link

backlog-helper bot commented Mar 9, 2023

While performing the daily checks some issues were found with this Pull Request.


New issue guide | Backlog management | Rules | Feedback

@jeffkreeftmeijer jeffkreeftmeijer merged commit 6462f80 into main Mar 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Logging backend
4 participants