Skip to content
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

feat(pipeline): Capture Trace flag #2293

Merged
merged 13 commits into from
Dec 16, 2021
5 changes: 5 additions & 0 deletions snuba/clickhouse/native.py
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ def execute(
settings: Optional[Mapping[str, Any]] = None,
types_check: bool = False,
columnar: bool = False,
capture_trace: bool = False,
) -> ClickhouseResult:
"""
Execute a clickhouse query with a single quick retry in case of
Expand Down Expand Up @@ -142,6 +143,7 @@ def execute_robust(
settings: Optional[Mapping[str, Any]] = None,
types_check: bool = False,
columnar: bool = False,
capture_trace: bool = False,
) -> ClickhouseResult:
"""
Execute a clickhouse query with a bit more tenacity. Make more retry
Expand All @@ -164,6 +166,7 @@ def execute_robust(
settings=settings,
types_check=types_check,
columnar=columnar,
capture_trace=capture_trace,
)
except (errors.NetworkError, errors.SocketTimeoutError, EOFError) as e:
# Try 3 times on connection issues.
Expand Down Expand Up @@ -308,6 +311,7 @@ def execute(
settings: Optional[Mapping[str, str]] = None,
with_totals: bool = False,
robust: bool = False,
capture_trace: bool = False,
) -> Result:
settings = {**settings} if settings is not None else {}

Expand All @@ -325,6 +329,7 @@ def execute(
with_column_types=True,
query_id=query_id,
settings=settings,
capture_trace=capture_trace,
),
with_totals=with_totals,
)
1 change: 1 addition & 0 deletions snuba/reader.py
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ def execute(
settings: Optional[Mapping[str, str]] = None,
with_totals: bool = False,
robust: bool = False,
capture_trace: bool = False,
) -> Result:
"""Execute a query."""
raise NotImplementedError
12 changes: 12 additions & 0 deletions snuba/request/request_settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,10 @@ def get_rate_limit_params(self) -> Sequence[RateLimitParameters]:
def add_rate_limit(self, rate_limit_param: RateLimitParameters) -> None:
pass

@abstractmethod
def get_capture_trace(self) -> bool:
pass


class HTTPRequestSettings(RequestSettings):
"""
Expand All @@ -67,6 +71,7 @@ def __init__(
parent_api: str = "<unknown>",
dry_run: bool = False,
legacy: bool = False,
capture_trace: bool = False,
) -> None:
super().__init__(referrer=referrer)
self.__turbo = turbo
Expand All @@ -76,6 +81,7 @@ def __init__(
self.__dry_run = dry_run
self.__legacy = legacy
self.__rate_limit_params: List[RateLimitParameters] = []
self.__capture_trace = capture_trace

def get_turbo(self) -> bool:
return self.__turbo
Expand All @@ -101,6 +107,9 @@ def get_rate_limit_params(self) -> Sequence[RateLimitParameters]:
def add_rate_limit(self, rate_limit_param: RateLimitParameters) -> None:
self.__rate_limit_params.append(rate_limit_param)

def get_capture_trace(self) -> bool:
return self.__capture_trace


class SubscriptionRequestSettings(RequestSettings):
"""
Expand Down Expand Up @@ -137,3 +146,6 @@ def get_rate_limit_params(self) -> Sequence[RateLimitParameters]:

def add_rate_limit(self, rate_limit_param: RateLimitParameters) -> None:
pass

def get_capture_trace(self) -> bool:
return False
4 changes: 4 additions & 0 deletions snuba/request/schema.py
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,10 @@ def generate_template(self) -> Any:
"dry_run": {"type": "boolean", "default": False},
# Flags if this a legacy query that was automatically generated by the SnQL SDK
"legacy": {"type": "boolean", "default": False},
# Flag to retrieve only ClickHouse tracing data. If this is True,
# the query execution should not return the results of the query, but
# return the tracing data of the query execution within ClickHouse
"capture_trace": {"type": "boolean", "default": False},
Comment on lines +136 to +139
Copy link
Contributor

Choose a reason for hiding this comment

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

I am not sure we want to expose this in the snuba api.
We cannot change the log handler there if that impacts the system logs or snuba logs, which means we would not be using this in that circumstance.
Also this may not work in HTTP and we want to move away from the TCP protocol for the Snuba queries, which may mean this will not be able to work on the snuba api.

I would suggest we remove it from there till we do not decide to expose it.

},
"additionalProperties": False,
},
Expand Down
1 change: 1 addition & 0 deletions snuba/web/db_query.py
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,7 @@ def execute_query(
query_settings,
with_totals=clickhouse_query.has_totals(),
robust=robust,
capture_trace=request_settings.get_capture_trace(),
)

timer.mark("execute")
Expand Down
2 changes: 2 additions & 0 deletions tests/clusters/fake_cluster.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ def execute(
settings: Optional[Mapping[str, Any]] = None,
types_check: bool = False,
columnar: bool = False,
capture_trace: bool = False,
) -> ClickhouseResult:
self.__queries.append(query)
return ClickhouseResult([[1]])
Expand All @@ -49,6 +50,7 @@ def execute(
settings: Optional[Mapping[str, Any]] = None,
types_check: bool = False,
columnar: bool = False,
capture_trace: bool = False,
) -> ClickhouseResult:
raise ServerExplodedException("The server exploded")

Expand Down
19 changes: 19 additions & 0 deletions tests/test_snql_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -596,6 +596,25 @@ def test_valid_columns_composite_query(self) -> None:
)
assert response.status_code == 200

def test_capture_trace_flag(self) -> None:
response = self.post(
"/events/snql",
data=json.dumps(
{
"query": """
MATCH (discover)
SELECT count() AS count
WHERE
timestamp >= toDateTime('2021-01-01') AND
timestamp < toDateTime('2022-01-01') AND
project_id IN tuple(5433960)
""",
"capture_trace": True,
}
),
)
assert response.status_code == 200

MATCH = "MATCH (e: events) -[grouped]-> (gm: groupedmessage)"
SELECT = "SELECT e.group_id, gm.status, avg(e.retention_days) AS avg BY e.group_id, gm.status"
WHERE = "WHERE e.project_id = 1 AND gm.project_id = 1"
Expand Down