-
Notifications
You must be signed in to change notification settings - Fork 22
Add telemetry related calls (for pipeline monitoring) #648
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
Changes from all commits
25d4800
6a25859
509eef4
d6e8495
127da45
6015161
31a5c15
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -46,13 +46,14 @@ | |||||||||||||||||||||||||||||||||||||
| from pydantic import BaseModel | ||||||||||||||||||||||||||||||||||||||
| from jose import jwt | ||||||||||||||||||||||||||||||||||||||
| from jose.exceptions import JWTError | ||||||||||||||||||||||||||||||||||||||
| from kernelci.api.models import ( | ||||||||||||||||||||||||||||||||||||||
| Node, | ||||||||||||||||||||||||||||||||||||||
| Hierarchy, | ||||||||||||||||||||||||||||||||||||||
| PublishEvent, | ||||||||||||||||||||||||||||||||||||||
| parse_node_obj, | ||||||||||||||||||||||||||||||||||||||
| KernelVersion, | ||||||||||||||||||||||||||||||||||||||
| EventHistory, | ||||||||||||||||||||||||||||||||||||||
| TelemetryEvent, | ||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||
| from .auth import Authentication | ||||||||||||||||||||||||||||||||||||||
| from .db import Database | ||||||||||||||||||||||||||||||||||||||
|
|
@@ -953,6 +954,324 @@ | |||||||||||||||||||||||||||||||||||||
| return JSONResponse(content=json_comp) | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| # ----------------------------------------------------------------------------- | ||||||||||||||||||||||||||||||||||||||
| # Telemetry of pipeline execution and other events(not node stuff). | ||||||||||||||||||||||||||||||||||||||
nuclearcat marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||
| # This is a separate collection from | ||||||||||||||||||||||||||||||||||||||
| # EventHistory since it may have a much higher volume and different | ||||||||||||||||||||||||||||||||||||||
| # query patterns and allows us to optimize indexes and storage | ||||||||||||||||||||||||||||||||||||||
| # separately. | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| @app.post('/telemetry', response_model=dict, tags=["telemetry"]) | ||||||||||||||||||||||||||||||||||||||
nuclearcat marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||
| async def post_telemetry( | ||||||||||||||||||||||||||||||||||||||
| events: List[dict], | ||||||||||||||||||||||||||||||||||||||
| current_user: User = Depends(get_current_user), | ||||||||||||||||||||||||||||||||||||||
| ): | ||||||||||||||||||||||||||||||||||||||
| """Bulk insert telemetry events. | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| Accepts a list of telemetry event dicts. Each event must have at | ||||||||||||||||||||||||||||||||||||||
| least 'kind' and 'runtime' fields. Events are validated against | ||||||||||||||||||||||||||||||||||||||
| the TelemetryEvent model before insertion. | ||||||||||||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||||||||||||
| metrics.add('http_requests_total', 1) | ||||||||||||||||||||||||||||||||||||||
| if not events: | ||||||||||||||||||||||||||||||||||||||
| raise HTTPException( | ||||||||||||||||||||||||||||||||||||||
| status_code=status.HTTP_400_BAD_REQUEST, | ||||||||||||||||||||||||||||||||||||||
| detail="Events list cannot be empty", | ||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||
| docs = [] | ||||||||||||||||||||||||||||||||||||||
| for event in events: | ||||||||||||||||||||||||||||||||||||||
| try: | ||||||||||||||||||||||||||||||||||||||
| obj = TelemetryEvent(**event) | ||||||||||||||||||||||||||||||||||||||
| except Exception as exc: | ||||||||||||||||||||||||||||||||||||||
| raise HTTPException( | ||||||||||||||||||||||||||||||||||||||
| status_code=status.HTTP_400_BAD_REQUEST, | ||||||||||||||||||||||||||||||||||||||
| detail=f"Invalid telemetry event: {exc}", | ||||||||||||||||||||||||||||||||||||||
| ) from exc | ||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+983
to
+989
|
||||||||||||||||||||||||||||||||||||||
| doc = obj.model_dump(by_alias=True) | ||||||||||||||||||||||||||||||||||||||
| doc.pop('_id', None) | ||||||||||||||||||||||||||||||||||||||
| docs.append(doc) | ||||||||||||||||||||||||||||||||||||||
| inserted_ids = await db.insert_many(TelemetryEvent, docs) | ||||||||||||||||||||||||||||||||||||||
| return {"inserted": len(inserted_ids)} | ||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+964
to
+994
|
||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| @app.get('/telemetry', response_model=PageModel, tags=["telemetry"]) | ||||||||||||||||||||||||||||||||||||||
| async def get_telemetry(request: Request): | ||||||||||||||||||||||||||||||||||||||
| """Query telemetry events with optional filters. | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| Supports filtering by any TelemetryEvent field, plus time range | ||||||||||||||||||||||||||||||||||||||
| via 'since' and 'until' parameters (ISO 8601 format). | ||||||||||||||||||||||||||||||||||||||
| Results are paginated (default limit=50). | ||||||||||||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||||||||||||
| metrics.add('http_requests_total', 1) | ||||||||||||||||||||||||||||||||||||||
| query_params = dict(request.query_params) | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| for pg_key in ['limit', 'offset']: | ||||||||||||||||||||||||||||||||||||||
| query_params.pop(pg_key, None) | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| since = query_params.pop('since', None) | ||||||||||||||||||||||||||||||||||||||
| until = query_params.pop('until', None) | ||||||||||||||||||||||||||||||||||||||
| if since or until: | ||||||||||||||||||||||||||||||||||||||
| ts_filter = {} | ||||||||||||||||||||||||||||||||||||||
| if since: | ||||||||||||||||||||||||||||||||||||||
| ts_filter['$gte'] = datetime.fromisoformat(since) | ||||||||||||||||||||||||||||||||||||||
| if until: | ||||||||||||||||||||||||||||||||||||||
| ts_filter['$lte'] = datetime.fromisoformat(until) | ||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+1015
to
+1018
|
||||||||||||||||||||||||||||||||||||||
| if since: | |
| ts_filter['$gte'] = datetime.fromisoformat(since) | |
| if until: | |
| ts_filter['$lte'] = datetime.fromisoformat(until) | |
| try: | |
| if since: | |
| ts_filter['$gte'] = datetime.fromisoformat(since) | |
| if until: | |
| ts_filter['$lte'] = datetime.fromisoformat(until) | |
| except ValueError as exc: | |
| raise HTTPException( | |
| status_code=status.HTTP_400_BAD_REQUEST, | |
| detail="Invalid 'since' or 'until' timestamp; expected ISO 8601 format.", | |
| ) from exc |
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.
done
nuclearcat marked this conversation as resolved.
Show resolved
Hide resolved
nuclearcat marked this conversation as resolved.
Show resolved
Hide resolved
nuclearcat marked this conversation as resolved.
Show resolved
Hide resolved
Copilot
AI
Feb 16, 2026
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.
Misleading or incorrect documentation. The docstring states "This is rule-based anomaly detection using thresholded empirical rates..." but this endpoint (get_telemetry_stats) only provides aggregated statistics without any anomaly detection logic. Anomaly detection is actually performed by the get_telemetry_anomalies endpoint. This docstring should describe the stats aggregation functionality, not anomaly detection.
Copilot
AI
Feb 16, 2026
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.
Missing error handling for datetime.fromisoformat(). If 'since' or 'until' parameters contain invalid ISO 8601 format strings, this will raise a ValueError that is not caught, resulting in an unhandled 500 error instead of a user-friendly 400 Bad Request. Wrap these calls in a try-except block and raise HTTPException with status 400.
| match_stage['ts'] = { | |
| **({'$gte': datetime.fromisoformat(since)} if since else {}), | |
| **({'$lte': datetime.fromisoformat(until)} if until else {}), | |
| since_dt = None | |
| until_dt = None | |
| try: | |
| if since: | |
| since_dt = datetime.fromisoformat(since) | |
| if until: | |
| until_dt = datetime.fromisoformat(until) | |
| except ValueError as exc: | |
| raise HTTPException( | |
| status_code=status.HTTP_400_BAD_REQUEST, | |
| detail="Invalid 'since' or 'until' datetime format; expected ISO 8601" | |
| ) from exc | |
| match_stage['ts'] = { | |
| **({'$gte': since_dt} if since_dt is not None else {}), | |
| **({'$lte': until_dt} if until_dt is not None else {}), |
Copilot
AI
Feb 16, 2026
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.
Duplicate database aggregation call. The same aggregation pipeline is executed twice on consecutive lines. Remove the first call on line 1127.
| results = await db.aggregate(TelemetryEvent, pipeline) |
nuclearcat marked this conversation as resolved.
Show resolved
Hide resolved
nuclearcat marked this conversation as resolved.
Show resolved
Hide resolved
Copilot
AI
Feb 16, 2026
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.
datetime.utcnow() is deprecated as of Python 3.12 in favor of datetime.now(timezone.utc). This method is scheduled for removal in a future Python version. Consider using datetime.now(timezone.utc) instead for timezone-aware datetime objects.
| since = datetime.utcnow() - timedelta(hours=hours) | |
| since = datetime.now(timezone.utc) - timedelta(hours=hours) |
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.
The aggregate method returns all results with length=None, which could lead to memory issues if the aggregation returns a large dataset. Consider adding a reasonable limit or documenting that callers should use appropriate $limit stages in their pipelines to avoid exhausting memory.