-
Notifications
You must be signed in to change notification settings - Fork 813
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
New method to post events from UDP packets to payloads #852
Conversation
} | ||
if date_happened is not None: | ||
event['date_happened'] = date_happened | ||
else: | ||
event['date_happened'] = int(time()) | ||
event['timestamp'] = event['date_happened'] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need both the date happened and a timestamp?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In templeton, timestamp is required, and its value used to fill in date_happened... But date_happened is set a default value of now in the agent aggregator, and then gives its value to timestamp
We could keep only the timestamp all along the way and let templeton set the date_happened (btw once date_happened is set in templeton, timestamp is removed. All this is confusing)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, so let's just send timestamp then since it seems to be using that in templeton. That also jives with how we submit events from agent checks, which just require a timestamp.
For your other note, I agree with you that having both a timestamp and date_happened is definitely confusing, but one is used for the Agent case and one for API events. We could consolidate, but we'd still have to support timestamp because we need to be backwards-compatible with event submissions using the older field.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok so now only a timestamp key is set. And in templeton date_happened is passed the timestamp value, then timestamp is deleted (as it was before)
Just don't know what to log when response from http connect Fixes: #852
Fixes: 852
Only timestamp now Fixes: # 852
It looks great! |
@@ -336,6 +349,7 @@ def init(config_path=None, use_watchdog=False, use_forwarder=False): | |||
non_local_traffic = c['non_local_traffic'] | |||
forward_to_host = c.get('statsd_forward_host') | |||
forward_to_port = c.get('statsd_forward_port') | |||
event_chunk_size = c.get('event_chunk_size') or None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you don't need the or None
here. The default value returned by get
will be None
Fixes: #852
New method to post events from UDP packets to payloads
Looks better this time.