Skip to content

Commit

Permalink
feat: better visibility (#2775)
Browse files Browse the repository at this point in the history
Signed-off-by: Tal <[email protected]>
  • Loading branch information
talboren authored Dec 15, 2024
1 parent c708d4e commit d0ff56e
Show file tree
Hide file tree
Showing 45 changed files with 1,864 additions and 982 deletions.
12 changes: 6 additions & 6 deletions .github/workflows/test-pr-e2e.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@ on:
workflow_dispatch:
pull_request:
paths:
- 'keep/**'
- 'keep-ui/**'
- 'tests/**'
- "keep/**"
- "keep-ui/**"
- "tests/**"

concurrency:
group: ${{ github.workflow }}-${{ github.head_ref }}
Expand Down Expand Up @@ -123,7 +123,7 @@ jobs:
# create the state directory
# mkdir -p ./state && chown -R root:root ./state && chmod -R 777 ./state
- name: Run e2e tests and report coverage
run: |
poetry run coverage run --branch -m pytest -s tests/e2e_tests/
Expand All @@ -147,9 +147,9 @@ jobs:

- name: Upload test artifacts on failure
if: always()
uses: actions/upload-artifact@v3
uses: actions/upload-artifact@v4.4.3
with:
name: test-artifacts
name: test-artifacts-my-artifacts-${{ matrix.db_type }}
path: |
playwright_dump_*.html
playwright_dump_*.png
Expand Down
6 changes: 4 additions & 2 deletions .github/workflows/test-pr.yml
Original file line number Diff line number Diff line change
Expand Up @@ -91,9 +91,11 @@ jobs:
run: poetry install --no-interaction --no-root --with dev

- name: Run unit tests and report coverage
env:
LOG_LEVEL: DEBUG
SQLALCHEMY_WARN_20: 1
run: |
poetry run coverage run --branch -m pytest -n auto --non-integration --ignore=tests/e2e_tests/
poetry run coverage run --branch -m pytest --timeout 20 -n auto --non-integration --ignore=tests/e2e_tests/
- name: Run integration tests and report coverage
run: |
Expand Down
25 changes: 13 additions & 12 deletions keep-ui/app/(keep)/alerts/alert-name.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,11 @@ import {
TicketIcon,
TrashIcon,
PencilSquareIcon,
Cog8ToothIcon,
// Cog8ToothIcon,
} from "@heroicons/react/24/outline";
import { Icon } from "@tremor/react";
import { AlertDto, AlertToWorkflowExecution } from "./models";
import { useWorkflowExecutions } from "utils/hooks/useWorkflowExecutions";
// import { useWorkflowExecutions } from "utils/hooks/useWorkflowExecutions";
import { useRouter } from "next/navigation";

interface Props {
Expand All @@ -22,7 +22,8 @@ export default function AlertName({
setTicketModalAlert,
}: Props) {
const router = useRouter();
const { data: executions } = useWorkflowExecutions();
// TODO: fix this so we can show which alert had workflow execution
// const { data: executions } = useWorkflowExecutions();

const handleNoteClick = () => {
if (setNoteModalAlert) {
Expand All @@ -38,9 +39,9 @@ export default function AlertName({
}
};

const relevantWorkflowExecution =
executions?.find((wf) => wf.alert_fingerprint === alert.fingerprint) ??
null;
const relevantWorkflowExecution: AlertToWorkflowExecution | null = null;
// executions?.find((wf) => wf.alert_fingerprint === alert.fingerprint) ??
// null;

const {
name,
Expand Down Expand Up @@ -131,29 +132,29 @@ export default function AlertName({
variant="solid"
/>
)}
{relevantWorkflowExecution && (
{/* {relevantWorkflowExecution && (
<Icon
icon={Cog8ToothIcon}
size="xs"
color={`${
relevantWorkflowExecution.workflow_status === "success"
? "green"
: relevantWorkflowExecution.workflow_status === "error"
? "red"
: "gray"
? "red"
: "gray"
}`}
tooltip={`${
relevantWorkflowExecution.workflow_status === "success"
? "Last workflow executed successfully"
: relevantWorkflowExecution.workflow_status === "error"
? "Last workflow execution failed"
: undefined
? "Last workflow execution failed"
: undefined
}`}
onClick={() => handleWorkflowClick(relevantWorkflowExecution)}
className="ml-1 cursor-pointer"
variant="solid"
/>
)}
)} */}
</div>
</div>
);
Expand Down
111 changes: 69 additions & 42 deletions keep/api/alert_deduplicator/alert_deduplicator.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
get_alerts_fields,
get_all_deduplication_rules,
get_all_deduplication_stats,
get_custom_deduplication_rules,
get_custom_deduplication_rule,
get_last_alert_hash_by_fingerprint,
update_deduplication_rule,
)
Expand All @@ -31,12 +31,16 @@

class AlertDeduplicator:

DEDUPLICATION_DISTRIBUTION_ENABLED = config(
"KEEP_DEDUPLICATION_DISTRIBUTION_ENABLED", cast=bool, default=True
)
CUSTOM_DEDUPLICATION_DISTRIBUTION_ENABLED = config(
"KEEP_CUSTOM_DEDUPLICATION_ENABLED", cast=bool, default=True
)

def __init__(self, tenant_id):
self.logger = logging.getLogger(__name__)
self.tenant_id = tenant_id
self.provider_distribution_enabled = config(
"PROVIDER_DISTRIBUTION_ENABLED", cast=bool, default=True
)
self.search_engine = SearchEngine(self.tenant_id)

def _apply_deduplication_rule(
Expand Down Expand Up @@ -91,13 +95,23 @@ def _apply_deduplication_rule(
},
)
alert.isPartialDuplicate = True
else:
self.logger.info(
"Alert is not deduplicated",
extra={
"alert_id": alert.id,
"fingerprint": alert.fingerprint,
"tenant_id": self.tenant_id,
"last_alert_hash_by_fingerprint": last_alert_hash_by_fingerprint,
},
)

return alert

def apply_deduplication(self, alert: AlertDto) -> bool:
# IMPOTRANT NOTE TO SOMEONE WORKING ON THIS CODE:
# apply_deduplication runs AFTER _format_alert, so you can assume that alert fields are in the expected format.
# you can also safe to assume that alert.fingerprint is set by the provider itself
# you are also safe to assume that alert.fingerprint is set by the provider itself

# get only relevant rules
rules = self.get_deduplication_rules(
Expand All @@ -122,26 +136,30 @@ def apply_deduplication(self, alert: AlertDto) -> bool:
"is_partial_duplicate": alert.isPartialDuplicate,
},
)
if alert.isFullDuplicate or alert.isPartialDuplicate:
# create deduplication event
create_deduplication_event(
tenant_id=self.tenant_id,
deduplication_rule_id=rule.id,
deduplication_type="full" if alert.isFullDuplicate else "partial",
provider_id=alert.providerId,
provider_type=alert.providerType,
)
# we don't need to check the other rules
break
else:
# create none deduplication event, for statistics
create_deduplication_event(
tenant_id=self.tenant_id,
deduplication_rule_id=rule.id,
deduplication_type="none",
provider_id=alert.providerId,
provider_type=alert.providerType,
)

if AlertDeduplicator.DEDUPLICATION_DISTRIBUTION_ENABLED:
if alert.isFullDuplicate or alert.isPartialDuplicate:
# create deduplication event
create_deduplication_event(
tenant_id=self.tenant_id,
deduplication_rule_id=rule.id,
deduplication_type=(
"full" if alert.isFullDuplicate else "partial"
),
provider_id=alert.providerId,
provider_type=alert.providerType,
)
# we don't need to check the other rules
break
else:
# create none deduplication event, for statistics
create_deduplication_event(
tenant_id=self.tenant_id,
deduplication_rule_id=rule.id,
deduplication_type="none",
provider_id=alert.providerId,
provider_type=alert.providerType,
)

return alert

Expand All @@ -166,11 +184,15 @@ def get_deduplication_rules(
self, tenant_id, provider_id, provider_type
) -> DeduplicationRuleDto:
# try to get the rule from the database
rules = get_custom_deduplication_rules(tenant_id, provider_id, provider_type)
rule = (
get_custom_deduplication_rule(tenant_id, provider_id, provider_type)
if AlertDeduplicator.CUSTOM_DEDUPLICATION_DISTRIBUTION_ENABLED
else None
)

if not rules:
if not rule:
self.logger.debug(
"No custom deduplication rules found, using deafult full deduplication rule",
"No custom deduplication rule found, using deafult full deduplication rule",
extra={
"provider_id": provider_id,
"provider_type": provider_type,
Expand All @@ -189,12 +211,10 @@ def get_deduplication_rules(
"tenant_id": tenant_id,
},
)
#
# check that at least one of them is full deduplication rule
full_deduplication_rules = [rule for rule in rules if rule.full_deduplication]

# if full deduplication rule found, return the rules
if full_deduplication_rules:
return rules
if rule.full_deduplication:
return [rule]

# if not, assign them the default full deduplication rule ignore fields
self.logger.info(
Expand All @@ -203,13 +223,8 @@ def get_deduplication_rules(
default_full_dedup_rule = self._get_default_full_deduplication_rule(
provider_id=provider_id, provider_type=provider_type
)
for rule in rules:
if not rule.full_deduplication:
self.logger.debug(
"Assigning default full deduplication rule ignore fields",
)
rule.ignore_fields = default_full_dedup_rule.ignore_fields
return rules
rule.ignore_fields = default_full_dedup_rule.ignore_fields
return [rule]

def _generate_uuid(self, provider_id, provider_type):
# this is a way to generate a unique uuid for the default deduplication rule per (provider_id, provider_type)
Expand Down Expand Up @@ -269,7 +284,11 @@ def get_deduplications(self) -> list[DeduplicationRuleDto]:
provider_id, provider_type = dd.provider_id, dd.provider_type
dd.id = self._generate_uuid(provider_id, provider_type)
# get custom deduplication rules
custom_deduplications = get_all_deduplication_rules(self.tenant_id)
custom_deduplications = (
get_all_deduplication_rules(self.tenant_id)
if AlertDeduplicator.CUSTOM_DEDUPLICATION_DISTRIBUTION_ENABLED
else []
)
# cast to dto
custom_deduplications_dto = [
DeduplicationRuleDto(
Expand Down Expand Up @@ -347,6 +366,14 @@ def get_deduplications(self) -> list[DeduplicationRuleDto]:

result = []
for dedup in final_deduplications:
self.logger.debug(
"Calculating deduplication stats",
extra={
"deduplication_rule_id": dedup.id,
"tenant_id": self.tenant_id,
"deduplication_stats": deduplication_stats,
},
)
key = dedup.id
full_dedup = deduplication_stats.get(key, {"full_dedup_count": 0}).get(
"full_dedup_count", 0
Expand Down Expand Up @@ -377,7 +404,7 @@ def get_deduplications(self) -> list[DeduplicationRuleDto]:
)
result.append(dedup)

if self.provider_distribution_enabled:
if AlertDeduplicator.DEDUPLICATION_DISTRIBUTION_ENABLED:
for dedup in result:
for pd, stats in deduplication_stats.items():
if pd == f"{dedup.provider_id}_{dedup.provider_type}":
Expand Down
Loading

0 comments on commit d0ff56e

Please sign in to comment.