Skip to content

Commit

Permalink
feat(queries): Store query calculation trigger in cache payload to ge…
Browse files Browse the repository at this point in the history
…t metrics (#23477)
  • Loading branch information
webjunkie authored Jul 8, 2024
1 parent 19b5ca9 commit 2b4e9a1
Show file tree
Hide file tree
Showing 7 changed files with 141 additions and 9 deletions.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
68 changes: 68 additions & 0 deletions frontend/src/queries/schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -524,6 +524,10 @@
"format": "date-time",
"type": "string"
},
"calculation_trigger": {
"description": "What triggered the calculation of the query, leave empty if user/immediate",
"type": "string"
},
"columns": {
"items": {},
"type": "array"
Expand Down Expand Up @@ -616,6 +620,10 @@
"format": "date-time",
"type": "string"
},
"calculation_trigger": {
"description": "What triggered the calculation of the query, leave empty if user/immediate",
"type": "string"
},
"columns": {
"items": {},
"type": "array"
Expand Down Expand Up @@ -703,6 +711,10 @@
"format": "date-time",
"type": "string"
},
"calculation_trigger": {
"description": "What triggered the calculation of the query, leave empty if user/immediate",
"type": "string"
},
"columns": {
"items": {},
"type": "array"
Expand Down Expand Up @@ -781,6 +793,10 @@
"format": "date-time",
"type": "string"
},
"calculation_trigger": {
"description": "What triggered the calculation of the query, leave empty if user/immediate",
"type": "string"
},
"error": {
"description": "Query error. Returned only if 'explain' or `modifiers.debug` is true. Throws an error otherwise.",
"type": "string"
Expand Down Expand Up @@ -855,6 +871,10 @@
"format": "date-time",
"type": "string"
},
"calculation_trigger": {
"description": "What triggered the calculation of the query, leave empty if user/immediate",
"type": "string"
},
"clickhouse": {
"description": "Executed ClickHouse query",
"type": "string"
Expand Down Expand Up @@ -978,6 +998,10 @@
"format": "date-time",
"type": "string"
},
"calculation_trigger": {
"description": "What triggered the calculation of the query, leave empty if user/immediate",
"type": "string"
},
"compare": {
"items": {
"additionalProperties": false,
Expand Down Expand Up @@ -1101,6 +1125,10 @@
"format": "date-time",
"type": "string"
},
"calculation_trigger": {
"description": "What triggered the calculation of the query, leave empty if user/immediate",
"type": "string"
},
"error": {
"description": "Query error. Returned only if 'explain' or `modifiers.debug` is true. Throws an error otherwise.",
"type": "string"
Expand Down Expand Up @@ -1165,6 +1193,10 @@
"format": "date-time",
"type": "string"
},
"calculation_trigger": {
"description": "What triggered the calculation of the query, leave empty if user/immediate",
"type": "string"
},
"error": {
"description": "Query error. Returned only if 'explain' or `modifiers.debug` is true. Throws an error otherwise.",
"type": "string"
Expand Down Expand Up @@ -1229,6 +1261,10 @@
"format": "date-time",
"type": "string"
},
"calculation_trigger": {
"description": "What triggered the calculation of the query, leave empty if user/immediate",
"type": "string"
},
"error": {
"description": "Query error. Returned only if 'explain' or `modifiers.debug` is true. Throws an error otherwise.",
"type": "string"
Expand Down Expand Up @@ -1293,6 +1329,10 @@
"format": "date-time",
"type": "string"
},
"calculation_trigger": {
"description": "What triggered the calculation of the query, leave empty if user/immediate",
"type": "string"
},
"error": {
"description": "Query error. Returned only if 'explain' or `modifiers.debug` is true. Throws an error otherwise.",
"type": "string"
Expand Down Expand Up @@ -1360,6 +1400,10 @@
"format": "date-time",
"type": "string"
},
"calculation_trigger": {
"description": "What triggered the calculation of the query, leave empty if user/immediate",
"type": "string"
},
"error": {
"description": "Query error. Returned only if 'explain' or `modifiers.debug` is true. Throws an error otherwise.",
"type": "string"
Expand Down Expand Up @@ -1424,6 +1468,10 @@
"format": "date-time",
"type": "string"
},
"calculation_trigger": {
"description": "What triggered the calculation of the query, leave empty if user/immediate",
"type": "string"
},
"error": {
"description": "Query error. Returned only if 'explain' or `modifiers.debug` is true. Throws an error otherwise.",
"type": "string"
Expand Down Expand Up @@ -1488,6 +1536,10 @@
"format": "date-time",
"type": "string"
},
"calculation_trigger": {
"description": "What triggered the calculation of the query, leave empty if user/immediate",
"type": "string"
},
"dateFrom": {
"type": "string"
},
Expand Down Expand Up @@ -1561,6 +1613,10 @@
"format": "date-time",
"type": "string"
},
"calculation_trigger": {
"description": "What triggered the calculation of the query, leave empty if user/immediate",
"type": "string"
},
"columns": {
"items": {},
"type": "array"
Expand Down Expand Up @@ -1643,6 +1699,10 @@
"format": "date-time",
"type": "string"
},
"calculation_trigger": {
"description": "What triggered the calculation of the query, leave empty if user/immediate",
"type": "string"
},
"columns": {
"items": {},
"type": "array"
Expand Down Expand Up @@ -3955,6 +4015,10 @@
"format": "date-time",
"type": "string"
},
"calculation_trigger": {
"description": "What triggered the calculation of the query, leave empty if user/immediate",
"type": "string"
},
"is_cached": {
"type": "boolean"
},
Expand Down Expand Up @@ -7761,6 +7825,10 @@
"format": "date-time",
"type": "string"
},
"calculation_trigger": {
"description": "What triggered the calculation of the query, leave empty if user/immediate",
"type": "string"
},
"error": {
"description": "Query error. Returned only if 'explain' or `modifiers.debug` is true. Throws an error otherwise.",
"type": "string"
Expand Down
2 changes: 2 additions & 0 deletions frontend/src/queries/schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1015,6 +1015,8 @@ interface CachedQueryResponseMixin {
timezone: string
/** Query status indicates whether next to the provided data, a query is still running. */
query_status?: QueryStatus
/** What triggered the calculation of the query, leave empty if user/immediate */
calculation_trigger?: string
}

type CachedQueryResponse<T> = T & CachedQueryResponseMixin
Expand Down
2 changes: 1 addition & 1 deletion posthog/caching/insight_cache.py
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ def update_cache(caching_state_id: UUID):
"last_refresh_queued_at": caching_state.last_refresh_queued_at,
}

tag_queries(team_id=insight.team_id, insight_id=insight.pk)
tag_queries(team_id=insight.team_id, insight_id=insight.pk, trigger="warming")
if dashboard:
tag_queries(dashboard_id=dashboard.pk)

Expand Down
8 changes: 5 additions & 3 deletions posthog/clickhouse/client/execute_async.py
Original file line number Diff line number Diff line change
Expand Up @@ -156,12 +156,14 @@ def execute_process_query(

query_status.error = True # Assume error in case nothing below ends up working

trigger = "chained" if "chained" in (query_status.labels or []) else ""
if trigger == "chained":
tag_queries(trigger="chaining")

pickup_time = datetime.datetime.now(datetime.UTC)
if query_status.start_time:
wait_duration = (pickup_time - query_status.start_time) / datetime.timedelta(seconds=1)
QUERY_WAIT_TIME.labels(
team=team_id, mode=("chained" if "chained" in (query_status.labels or []) else None)
).observe(wait_duration)
QUERY_WAIT_TIME.labels(team=team_id, mode=trigger).observe(wait_duration)

try:
tag_queries(client_query_id=query_id, team_id=team_id, user_id=user_id)
Expand Down
19 changes: 14 additions & 5 deletions posthog/hogql_queries/query_runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
from posthog.cache_utils import OrjsonJsonSerializer
from posthog.caching.utils import is_stale, last_refresh_from_cached_result, ThresholdMode, cache_target_age
from posthog.clickhouse.client.execute_async import enqueue_process_query_task, get_query_status, QueryNotFoundError
from posthog.clickhouse.query_tagging import tag_queries
from posthog.clickhouse.query_tagging import tag_queries, get_query_tag_value
from posthog.hogql import ast
from posthog.hogql.constants import LimitContext
from posthog.hogql.context import HogQLContext
Expand Down Expand Up @@ -68,7 +68,7 @@
QUERY_CACHE_HIT_COUNTER = Counter(
"posthog_query_cache_hit_total",
"Whether we could fetch the query from the cache or not.",
labelnames=[LABEL_TEAM_ID, "cache_hit"],
labelnames=[LABEL_TEAM_ID, "cache_hit", "trigger"],
)

EXTENDED_CACHE_AGE = timedelta(days=1)
Expand Down Expand Up @@ -450,15 +450,22 @@ def handle_cache_and_async_logic(
)

if self.is_cached_response(cached_response_candidate):
assert isinstance(cached_response, CachedResponse)
cached_response.cache_target_age = self.cache_target_age(cached_response)

if not self._is_stale(cached_response):
QUERY_CACHE_HIT_COUNTER.labels(
team_id=self.team.pk,
cache_hit="hit",
trigger=cached_response.calculation_trigger or "",
).inc()
# We have a valid result that's fresh enough, let's return it
QUERY_CACHE_HIT_COUNTER.labels(team_id=self.team.pk, cache_hit="hit").inc()
cached_response.query_status = self.get_async_query_status(cache_key=cache_key)
return cached_response

QUERY_CACHE_HIT_COUNTER.labels(team_id=self.team.pk, cache_hit="stale").inc()
QUERY_CACHE_HIT_COUNTER.labels(
team_id=self.team.pk, cache_hit="stale", trigger=cached_response.calculation_trigger or ""
).inc()
# We have a stale result. If we aren't allowed to calculate, let's still return it
# – otherwise let's proceed to calculation
if execution_mode == ExecutionMode.CACHE_ONLY_NEVER_CALCULATE:
Expand All @@ -478,7 +485,7 @@ def handle_cache_and_async_logic(
cached_response.query_status = self.get_async_query_status(cache_key=cache_key)
return cached_response
else:
QUERY_CACHE_HIT_COUNTER.labels(team_id=self.team.pk, cache_hit="miss").inc()
QUERY_CACHE_HIT_COUNTER.labels(team_id=self.team.pk, cache_hit="miss", trigger="").inc()
# We have no cached result. If we aren't allowed to calculate, let's return the cache miss
# – otherwise let's proceed to calculation
if execution_mode == ExecutionMode.CACHE_ONLY_NEVER_CALCULATE:
Expand Down Expand Up @@ -525,6 +532,8 @@ def run(
"cache_key": cache_key,
"timezone": self.team.timezone,
}
if get_query_tag_value("trigger"):
fresh_response_dict["calculation_trigger"] = get_query_tag_value("trigger")
fresh_response = CachedResponse(**fresh_response_dict)

# Don't cache debug queries with errors and export queries
Expand Down
Loading

0 comments on commit 2b4e9a1

Please sign in to comment.