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

Implemented support for ServiceChecks #5

Closed
wants to merge 13 commits into from

Conversation

jovanbrakus
Copy link
Contributor

Support for ServiceChecks is implemented based on DataDog wire protocol specification.

@truthbk truthbk added this to the triage milestone Jan 15, 2016
@truthbk
Copy link
Member

truthbk commented Jan 15, 2016

@jovanbrakus sorry for the slow response, I realize you sbumitted this ages ago! We'll review this very shortly, thanks a lot for your patience! 👍

@seiffert
Copy link
Contributor

I stumbled upon the fact that event texts as well as service check messages can contain \n characters which need to be escaped (\\n). I just submitted a PR (#8) for the event texts and wanted to notify you about this. I haven't used service checks yet, but from the python client library, I can tell that the messages need to be escaped, too.

See https://github.com/DataDog/datadogpy/blob/b5f7d6d2748fad61b2e060768ce68e3b76d2c928/datadog/dogstatsd/base.py#L259-L263

@jovanbrakus
Copy link
Contributor Author

Yup, agreed.
Will do that soon.

@truthbk
Copy link
Member

truthbk commented Feb 12, 2016

Just a quick note, #8 has now been merged.

@jovanbrakus
Copy link
Contributor Author

I added escaping of '\n' and 'm:' along with tests that show functionality...

@truthbk
Copy link
Member

truthbk commented Mar 11, 2016

@jovanbrakus could you please rebase to the latest master?

@@ -266,6 +266,76 @@ func TestEvents(t *testing.T) {
}
}

func TestServiceChecks(t *testing.T) {
Copy link
Member

Choose a reason for hiding this comment

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

Thank you so much for writing this! ❤️

@truthbk
Copy link
Member

truthbk commented Mar 11, 2016

Looks solid, if you could please add the unicode message test, rebase and squash, we should then be good to merge!

Thanks so much for your contrib!

@jovanbrakus
Copy link
Contributor Author

Added unicode test and rebased it to latest master.

@truthbk
Copy link
Member

truthbk commented Apr 15, 2016

@jovanbrakus I think you accidentally rebased on the wrong branch or branched off the wrong one, so we've got a few conflicts... Anyways I created a branch (jaime/svchk_support @ https://github.com/DataDog/datadog-go/compare/jaime/svchk_support) with your commits and no conflicts, if you want to git reset --hard origin jaime/svchk_support) and give it a last pass?

@jovanbrakus
Copy link
Contributor Author

Haven't seen your msg before.... Branch svchk_support looks fine.

@diasjorge
Copy link

hi guys any chance this might get merged soon? we'd really like to use this functionality. Please :)

@jovanbrakus
Copy link
Contributor Author

jovanbrakus commented Aug 5, 2016

We're using ServiceChecks in production for months... but we would also like to have it merged in.

@diasjorge diasjorge mentioned this pull request Aug 12, 2016
@jovanbrakus
Copy link
Contributor Author

Closing this as it's been merged on another PR

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

Successfully merging this pull request may close these issues.

None yet

5 participants