Add log target and l2met transform

[stevenharman]

This builds atop #20. If/when it's merged, I'll rebase these changes atop main to make these changes more clear.

This PR adds a new LogTarget, and splits out Formatters and Transforms into their own objects, so they can be mixed and matched. The LogTarget defaults to using the Ruby stdlib ::Logger pointed at $stdout, using an Logfmt formatter. We can then use the new L2Met transform to get the metrics in our logs, exactly as we need them!

Puma::Plugin::Telemetry::Targets::LogTarget.new(transform: :l2met).call('queue.size': 42, load: 0.3)
#=> I, [2024-05-02T15:40:08.222977 #51368]  INFO -- : sample#queue.size=42 sample#load=0.3 name="Puma::Plugin::Telemetry"

# Or, JSON-ify them
Puma::Plugin::Telemetry::Targets::LogTarget.new(transform: :l2met, formatter: :json).call('queue.size': 42, load: 0.3)
#=> I, [2024-05-02T15:42:27.037245 #51368]  INFO -- : {"sample#queue.size":42,"sample#load":0.3,"name":"Puma::Plugin::Telemetry"}

These also work with the IOTarget:

Puma::Plugin::Telemetry::Targets::IOTarget.new(transform: :l2met, formatter: :logfmt).call('queue.size': 42, load: 0.3)
#=> sample#queue.size=42 sample#load=0.3 name="Puma::Plugin::Telemetry"

The default formatter: and transform: options for IOTarget keep it backward compatible with the way that target worked before.

I've tried to make each step along the way an atomic commit. It might be helpful to quickly look commit-by-commit if you're curious. Or not - most of the churn is just reorganizing the code that was here.

Phew! Thanks so much for this tooling, and for your consideration.

[driv3r]

I'll try to properly review tomorrow morning 👍

[unsign3d]

This is a bit of a big PR for a quick review, I will quickly create a ticket for ourselves to go through it and test it.
Thank you for contributing to this tool :)

[unsign3d]

I just realised that I removed @driv3r from the reviewer.
Feel free to review, that was an accident (and I kinda didn't even understand how I did it) XD

[stevenharman]

@unsign3d and @driv3r I've rebased this branch on main, which slims this down to just the behavior changes - all of the Gem cleanup is now part of main. Hopefully that makes this a little easier to review. 😄

[stevenharman]

This does keep things backward compatible, though I wonder if it's a bit surprising and changing the default, as part of a breaking-change 2.0 release, might be the way to go?

I could see the :noop transform being a sensible default. Then only if you're using CloudWatch Log Metrics, or L2Met, would you need to opt for a different transform. Thoughts?

[unsign3d]

I'm not sure I fully like the name noop, I think passthrough might be a better option for naming, but I'm really open minded about this

[stevenharman]

I don't really love it either. :passthrough seems a reasonable alternative to me. Anyone have other alternatives or thoughts?

[stevenharman]

Any further thoughts? I'm happy to rename to :passthrough; it is a bit more explicit of how it's not doing anything. 😄

[unsign3d]

First of all, thanks again for the contribution 👊

I really like the modularity and concepts you've introduced here. However, I feel this PR is bringing in quite a few changes, and I'm not fully sold on some of them yet.

If you're open for it, I would propose you to split this PR into 3 separate PRs:

• A PR that does the refactoring and exposes the noop classes as base classes, I feel that this would bring major advantages to people if they want to create new formatters and transformers, in this case you could leverage our codebase without needing to fork necessarily
• A PR that adds the logfmt and logger as possible options, based on the base classes (I feel this would be a good example that people could build on top)
• A PR that adds the l2met transform based on the base class, and check if it makes more sense as a plugin or as an upstream option

I think this contribution would make it for an awesome base, and people could start using their transformers without forking the code, and without constant rebasing, therefore reducing overall maintenance.

Does this approach make sense to you? I'm happy to discuss further!

[stevenharman]

@unsign3d yeah, if you'd rather see it broken up (1 & 2) and then potentially pass on the l2met formatter (3), that's fine. I'll break it up and open new PRs.

FWIW, if this was accepted my plan was to then work on adding the right amount of autoload so these extra transforms and formatters were only loaded into memory if/when they were used. That way having some extra ones (like l2met) wouldn't really have any impact to users.

[unsign3d]

@stevenharman that sounds good :)
What I'm not convinced is the maintenance of formatters after they become mainline, especially the l2met that we are not using and I'm not really sure how it works...
I'm leaning towards the idea of having another repo / gem with that formatter as a plugin, so that it can be developed independently and with more knowledge of the l2met platform that my team currently has 🤔
But I'm still open minded if you see more benefit in having that mainline instead of a plugin

[stevenharman]

Happy to take this to a different medium (GitHub Discussions?), but the gist on L2Met is it was primarily created/evolved as a way to emit metrics via log streams. The largest use case I know of is Librato (now SolarWinds), and it's pretty much the current standard for anything deployed to Heroku. That happens to be where all of our stuff is deployed, and we're currently using Librato. Hence, the need/desire for these logs to be emitted in L2Met format.

Does it belong in the main repo? I don't have a particularly strong opinion either way. That said, it's extremely stable (and pretty much in maintenance mode), and I'd expect very little carrying cost for this repo. It's also such a simple transform that it's trivial to extract. So… 🤷

[unsign3d]

Let's start by pushing it to upstream, and then if it becomes too much maintenance we can create a new major and move it into a plugin :)

[stevenharman]

Closing in favor of #25 and #26.