Expose new transforms and formatters design - #25
Conversation
|
@unsign3d Sorry for the long delay - summer arrived here, the kids are out of school, and we've been doing our best to get out and enjoy it. This is the first of 2-3 PRs to rebuild what we had in #21. Once this is merged I'll get to work on adding the Thank you! |
fc38c6e to
cca4954
Compare
|
@unsign3d 👋 It's been a while and we're still running these changes in production, with great success. Are you still open to pulling them in? Thanks! |
Making the formatter available outside the IOTarget means other targets can also use it. This will be helpful when I add a LogTarget. This also adds some very basic tests (for my own sanity) for the IOTarget and JSONFormatter classes.
As it was, the formatters were doing both transforms on the keys (specific to a particular service) AND formatting the telemetry data. By separating those responsibilities, we can mix and match different Targets, Formatters, and Transforms to get the output needed for different consumers.
These are useful for custom targets that might not need/want any transforms for formatting, or to mix-and-match. For example, image a custom `LogTarget` that can be configured with a `:logger` option. It could be that the logger itself already knows how to format they telemetry data, (e.g., into key/value pairs), and so we don't want/need the Target to do any formatting. This allows this plugin's config to look like this: ```ruby Puma::Plugin::Telemetry.configure do |config| config.add_target(CustomLogTarget, logger: Logfmt.logger, transform: :cloud_watch, formatter: :noop) end ```
cca4954 to
f815fd8
Compare
|
Hello. 👋 I'm not sure what the current status of this plugin/Gem is, but we have been running it, including the modifications in #21 (broken up as incremental changes in #25, #26, and #37). Anyhow, we are quite interested in keeping this repo alive as we depend on it for our high-traffic production workloads. I'm happy talk through these changes (there's a good bit of context in #21) but the idea is to make the plugin more extensible by others, like my teams, so we'd not need to fork/rebase constantly to keep up to date. With all of that said, I'd be happy to help with maintaining this Gem if you'd like a hand. Either way, what do you think about starting to roll these changes in, giving us a solid base for user-extensible formatters and transforms? Thanks! |
There was a problem hiding this comment.
Pull request overview
This PR extracts formatting/transformation concerns into composable formatter: and transform: options via a new BaseFormattingTarget, and updates the IO target/docs to support mix-and-match formatting/transforms while keeping IOTarget defaults backward compatible.
Changes:
- Introduces
BaseFormattingTargetto centralizeformatter:/transform:selection and wiring for targets. - Extracts CloudWatch key transformation and JSON formatting into discrete transform/formatter objects, plus passthrough variants.
- Adds specs and updates README/fixtures/changelog to document and demonstrate the new options.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| lib/puma/plugin/telemetry/targets/base_formatting_target.rb | New base target class that resolves and exposes formatter/transform callables. |
| lib/puma/plugin/telemetry/targets/io_target.rb | Refactors IO target to use BaseFormattingTarget and apply transform+formatter pipeline. |
| lib/puma/plugin/telemetry/transforms/cloud_watch_transform.rb | Adds CloudWatch transform extracted from prior IO target behavior. |
| lib/puma/plugin/telemetry/transforms/passthrough_transform.rb | Adds passthrough transform option. |
| lib/puma/plugin/telemetry/formatters/json_formatter.rb | Adds standalone JSON formatter option. |
| lib/puma/plugin/telemetry/formatters/passthrough_formatter.rb | Adds passthrough formatter option. |
| spec/puma/plugin/telemetry/targets/io_target_spec.rb | Adds coverage for IO target writing formatted telemetry to IO. |
| spec/puma/plugin/telemetry/transforms/cloud_watch_transform_spec.rb | Adds coverage for CloudWatch transform key behavior. |
| spec/puma/plugin/telemetry/transforms/passthrough_transform_spec.rb | Adds coverage for passthrough transform behavior. |
| spec/puma/plugin/telemetry/formatters/json_formatter_spec.rb | Adds coverage for JSON formatter output. |
| spec/puma/plugin/telemetry/formatters/passthrough_formatter_spec.rb | Adds coverage for passthrough formatter behavior. |
| spec/fixtures/sockets.rb | Updates fixture config to explicitly use passthrough transform with custom formatter. |
| README.md | Documents IO target options and example usage updates. |
| CHANGELOG.md | Updates changelog entries for new formatter/transform options and normalizes some headings. |
Suppressed comments (1)
CHANGELOG.md:73
???in a release heading looks like an accidentally-committed placeholder. If the date is unknown, consider omitting it entirely (or replacing with an actual date).
## [1.1.0 Alpha] ???
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…_formatters # Conflicts: # CHANGELOG.md
- Rename CloudWatchTranform to CloudWatchTransform. - Remove an unused require from the CloudWatch transform. - Raise NotImplementedError from the abstract target call method. - Fix typos in the passthrough specs.
Add a spec for the :passthrough formatter mapping on the IO target, and note the removed IOTarget::JSONFormatter constant in the changelog.
|
Thank you so much for adding this! It looks good to me, but I'd also like to get a review from @unsign3d |
|
Thank you so much! Released in https://github.com/babbel/puma-plugin-telemetry/releases/tag/v1.2.0 |
This work is an extraction of some of #21.
It includes a new base class (
BaseFormattingTarget) thatTargets can opt into, giving them access to theformatter:andtransform:options. It also extracts acloud_watchtransform andjsonformatter into discrete options, which can be mixed-and-matched with new:passthroughformatter and transform.This also keeps the existing
IOTargetbackwards compatible by defaulting toformatter: :jsonandtransform: :cloud_watch.It might be helpful to look at the changes/docs in the
READMEto get an idea of how these would be used.