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.

Sign up for GitHub

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

Jump to bottom

Http check additions #375

Merged
merged 5 commits into from
Feb 22, 2013
Merged

Http check additions #375

Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
d008f59
whitespace
dcrosta Jan 22, 2013
c111602
add HTTP body (if present) to http_check alert messages
dcrosta Feb 14, 2013
c856a43
only include http response body if there was a response
dcrosta Feb 15, 2013
9bed5e2
http_check: users must opt in to getting HTTP response content in alerts
dcrosta Feb 20, 2013
06c6976
http_check: don't escape HTML, web UI does this
dcrosta Feb 22, 2013
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
http_check: don't escape HTML, web UI does this
  • Loading branch information
@dcrosta
dcrosta committed Feb 22, 2013
commit 06c6976e8ff4d09c48d44fcc4d79aa2ec54cf1e5
1 change: 0 additions & 1 deletion checks.d/http_check.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,6 @@ def _create_status_event(self, status, msg, instance):
# truncate and html-escape content
if len(content) > 200:
content = content[:197] + '...'
content = content.replace('<', '&lt;').replace('>', '&gt;')

msg = "%d %s\n\n%s" % (code, reason, content)
Copy link
Contributor

Choose a reason for hiding this comment

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

last thing before we merge.
Why do you need this line ?
With this line the created event on datadog is full of < instead of proper < >.
It will be good to merge once it's removed! Thanks

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Which line?

If you mean L98: I wasn't sure if datadog will escape the HTML on display or not, and wanted to be safe. If it does do this, then, yes, we should remove it.

If you mean L100 -- then I'm not sure I understand the question.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes L98. Sorry I put the comment on the wrong line :$

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK. And DataDog's web UI escapes the HTML (including possibly malformed HTML due to truncation) properly?

Copy link
Contributor

Choose a reason for hiding this comment

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

yes

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

msg = msg.rstrip()
Expand Down