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

Feature/improve appsignal to support tagging #2

Merged

Conversation

Vuta
Copy link
Contributor

@Vuta Vuta commented Oct 5, 2020

background

We want to improve Appsignal logging by using tags for metric

design

  • Check if the payload hash is in correct format (needs value and tags key), else returns failure result.
  • Fallback to previous implementation if payload value is not a hash

test cases

  • EventTracer.info action: 'Action', message: 'Message', appsignal: { increment_counter: { counter_1: 1, counter_2: 2 } } works as usual
  • Tags can be added to metric with the format { name: { value: value, tags: hash } }
    Example:
    EventTracer.info(
      action: 'Action',
      message: 'Message',
      appsignal: {
        increment_counter: {
        counter_1: { value: 1, tags: { region: 'eu' } }
     }})
  • Failure returns if tags are in invalid format (missing value and tags)

@Vuta Vuta force-pushed the feature/improve_appsignal_to_support_tagging branch from 2dfa6c8 to f54f0b0 Compare October 5, 2020 11:53
@hieuk09 hieuk09 self-assigned this Oct 6, 2020
@hieuk09
Copy link
Collaborator

hieuk09 commented Oct 8, 2020

@Vuta could you bump the gem to 0.1.3. The changes look good to me.

@melvrickgoh
Copy link
Collaborator

@Vuta can u help to update this branch to incl. 0.2? I'll help to review the GH logs too

Copy link
Collaborator

@hieuk09 hieuk09 left a comment

Choose a reason for hiding this comment

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

  • tag is collected correctly

@Vuta could you bump this gem to 0.2.1?

@hieuk09 hieuk09 merged commit d4a566a into Kaligo:master Nov 15, 2020
@Vuta Vuta deleted the feature/improve_appsignal_to_support_tagging branch November 15, 2020 04:18
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.

4 participants