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

Add GraphiteDB tags formatter #818

Merged

Conversation

tuyndoan
Copy link
Contributor

Add GraphiteDB Tags Formatter

Description

Add support for GraphiteDB tags formatting in StatsD metrics. The new formatter allows tags to be placed right after the bucket name using GraphiteDB's specific format: ;tag1=value1;tag2;tag3=value3.

Changes

  • Add new GraphiteDbTagsFormatter class implementing StatsD tags formatting for GraphiteDB
  • Add corresponding unit tests to validate the formatter behavior
  • The formatter uses the following configuration:
    • Prefix: ";"
    • No suffix
    • Tags are placed immediately after metric name (non-trailing)
    • Tags separator: ";"
    • Key-value separator: "="

Example Outputs

# Counter with tags
prefix.bucket;foo=bar;empty;lorem=ipsum:128|c

# Timing with tags and sampling rate
prefix.bucket;foo=bar;empty;lorem=ipsum:128|ms|@0.5

# Gauge with floating point value
prefix.bucket;foo=bar;empty;lorem=ipsum:128.5|g

@tuyndoan tuyndoan requested a review from a team as a code owner January 15, 2025 04:20
Copy link
Member

@martincostello martincostello left a comment

Choose a reason for hiding this comment

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

Could you bump the version to 5.1.0 please?

<VersionPrefix>5.0.2</VersionPrefix>

Otherwise this looks good - thanks for the contribution!

src/JustEat.StatsD/PublicAPI.Shipped.txt Outdated Show resolved Hide resolved
/// <summary>
/// Gets a GraphiteDB tags formatter.
/// </summary>
public static IStatsDTagsFormatter GraphiteDb => new GraphiteDbTagsFormatter();
Copy link
Member

Choose a reason for hiding this comment

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

This highlights that we should have probably made these singletons via { get; } = ... rather than creating a new instance on every access.

I'll fix that in a separate PR after this is merged.

@martincostello martincostello added this to the v5.1.0 milestone Jan 15, 2025
Copy link

codecov bot commented Jan 15, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 84.06%. Comparing base (ec72d82) to head (441588f).
Report is 3 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #818      +/-   ##
==========================================
+ Coverage   83.82%   84.06%   +0.23%     
==========================================
  Files          28       29       +1     
  Lines         674      684      +10     
  Branches      151      151              
==========================================
+ Hits          565      575      +10     
  Misses         67       67              
  Partials       42       42              
Flag Coverage Δ
linux 83.04% <100.00%> (-0.35%) ⬇️
macos 83.33% <100.00%> (+0.24%) ⬆️
windows 83.47% <100.00%> (+0.24%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

@martincostello martincostello merged commit d10b4b8 into justeattakeaway:main Jan 15, 2025
12 of 13 checks passed
martincostello added a commit that referenced this pull request Jan 15, 2025
Make formatters singletons.

See #818 (comment).
martincostello added a commit that referenced this pull request Jan 15, 2025
Make formatters singletons.

See #818 (comment).
@martincostello
Copy link
Member

Thanks again for your contribution @tuyndoan - your changes are available in JustEat.StatsD v5.1.0 📦 which is now available from NuGet.org 🙇

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants