Skip to content

Update SirCAL logging #786

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

Merged
merged 1 commit into from
Feb 9, 2021
Merged

Update SirCAL logging #786

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
17 changes: 7 additions & 10 deletions sir_complainsalot/delphi_sir_complainsalot/run.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,14 +37,6 @@ def run_module():
params["sources"], params.get("grace", 0), LOGGER))

if len(complaints) > 0:
for complaint in complaints:
LOGGER.critical(event="signal out of SLA",
Copy link
Contributor

@sgsmob sgsmob Feb 9, 2021

Choose a reason for hiding this comment

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

Why does this need to be moved?

Copy link
Contributor Author

@benjaminysmith benjaminysmith Feb 9, 2021

Choose a reason for hiding this comment

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

Old: each line-level complaint was being logged one at a time (one line per type/source/signal/geo combo)
New: only the aggregated complaint is logged (one line per type/source; signal/geo aggregated)

(this makes it much more useful for alerting)

message=complaint.message,
data_source=complaint.data_source,
signal=complaint.signal,
geo_types=complaint.geo_types,
last_updated=complaint.last_updated.strftime("%Y-%m-%d"))
Copy link
Contributor

@sgsmob sgsmob Feb 9, 2021

Choose a reason for hiding this comment

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

This argument is dropped below. Was that intentional?

Copy link
Contributor Author

@benjaminysmith benjaminysmith Feb 9, 2021

Choose a reason for hiding this comment

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

Yeah, in the new code it's been pulled directly into the aggregated message.


report_complaints(complaints, slack_notifier)

elapsed_time_in_seconds = round(time.time() - start_time, 2)
Expand All @@ -60,12 +52,12 @@ def split_complaints(complaints, n=49):
def report_complaints(all_complaints, slack_notifier):
"""Post complaints to Slack."""
if not slack_notifier:
LOGGER.info("(dry-run)")
print("(dry-run)")
return

for complaints in split_complaints(all_complaints):
blocks = format_complaints_aggregated_by_source(complaints)
LOGGER.info(f"blocks: {len(blocks)}")
print(f"blocks: {len(blocks)}")
slack_notifier.post_message(blocks)

def get_maintainers_block(complaints):
Expand Down Expand Up @@ -108,6 +100,11 @@ def message_for_source(complaint):
signal=complaint.signal,
geo_types=", ".join(complaint.geo_types))

LOGGER.critical(event="Signal out of SLA",
message=message,
data_source=source,
signal_and_geo_types=signal_and_geo_types)

blocks.extend([
{
"type": "divider"
Expand Down