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

feat(client): Add support for tags #922

Merged
merged 4 commits into from
May 21, 2024
Merged

feat(client): Add support for tags #922

merged 4 commits into from
May 21, 2024

Conversation

ryanrishi
Copy link
Contributor

This change adds support for tagging metrics. This is helpful when there are multiple instances of the service running in the cloud, eg. for tagging based on availability zone, region, version, etc.

Tags are not part of the StatsD protocol, but are generally accepted by StatsD servers in the following form: <metric>|#key1:value1,key2,value2. For further examples, see Datadog's DogStatsD protocol and NewRelic's metric format.

⚙️ Types of changes

What types of changes does your code introduce? Put an x in the boxes that apply

  • New feature (non-breaking change which adds functionality)
  • Bugfix (non-breaking change which fixes an issue)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation Update (if none of the other choices apply)
  • Refactor

@ryanrishi
Copy link
Contributor Author

@simonecorsi are you able to take a look at this?

@simonecorsi
Copy link
Collaborator

simonecorsi commented May 16, 2024

@simonecorsi are you able to take a look at this?

@ryanrishi Thank you for tagging me, I didn't see the notification the first time, we've been a bit busy :) Let me do some quick checks with the team and we'll get back to this!

@simonecorsi simonecorsi added the enhancement New feature or request label May 16, 2024
Copy link
Contributor

@antoniomuso antoniomuso left a comment

Choose a reason for hiding this comment

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

LGTM. Only one particular performance tip.

src/index.ts Outdated Show resolved Hide resolved
src/index.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@dnlup dnlup left a comment

Choose a reason for hiding this comment

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

Great work @ryanrishi , thank you!

Copy link
Contributor

@antoniomuso antoniomuso left a comment

Choose a reason for hiding this comment

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

LGTM, could you rewrite the commits using Conventional Commit?

@ryanrishi
Copy link
Contributor Author

@antoniomuso thanks, rewrote the commits to conform to conventional commit

@simonecorsi simonecorsi self-requested a review May 21, 2024 07:53
@simonecorsi simonecorsi merged commit c92a21e into immobiliare:main May 21, 2024
6 checks passed
@simonecorsi
Copy link
Collaborator

🎉 This PR is included in version 5.1.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@ryanrishi
Copy link
Contributor Author

Thanks @simonecorsi ! To get this in https://github.com/immobiliare/fastify-metrics, will dependabot open that PR automatically or should I open a PR to bump the version there?

@ryanrishi
Copy link
Contributor Author

Thanks @simonecorsi ! To get this in https://github.com/immobiliare/fastify-metrics, will dependabot open that PR automatically or should I open a PR to bump the version there?

Created immobiliare/fastify-metrics#613

@ryanrishi ryanrishi deleted the feature/add-tags branch June 16, 2024 19:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request released
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants