feat(pipeline): Capture Trace flag

[rahul-kumar-saini]

Overview

• We need a flag to know whether or not to capture the ClickHouse trace
• This comes in through RequestSettings and gets propagated through to query execution
• The flag currently isn't used to do anything but it is required for future changes to capture and return ClickHouse tracing data when needed through the admin portal

Changes

• Added capture_trace flag to RequestSettings and schema
• Updated execution pipeline to propagate flag through to ClickHouse execute() methods

[volokluev]

So this means that technically any snuba API call can request a trace. Is that what we want? Are there any dangers in exposing this functionality to all callers?

[onewland]

Is there any reason to separate the change where it gets wired through?

[rahul-kumar-saini]

Right, the flag needs to be passed via the API request but I'm unsure how to make it so only the admin portal can use it.

@onewland This doesn't need to be a separate PR but I'd like to figure out how to limit the exposure of the flag in the API and we can work on using it later.

[evanh]

Could you add a test for this? Maybe add a test to one of the *_api.py tests to make sure that adding the field doesn't break anything.

[codecov-commenter]

Codecov Report

Merging #2293 (6a4f860) into master (b9198f8) will decrease coverage by 0.00%.
The diff coverage is 90.90%.

❗ Current head 6a4f860 differs from pull request most recent head e3bd4e7. Consider uploading reports for the commit e3bd4e7 to get more accurate results

@@            Coverage Diff             @@
##           master    #2293      +/-   ##
==========================================
- Coverage   92.96%   92.96%   -0.01%     
==========================================
  Files         556      556              
  Lines       25436    25447      +11     
==========================================
+ Hits        23646    23656      +10     
- Misses       1790     1791       +1     

Impacted Files	Coverage Δ
snuba/clickhouse/native.py	79.10% <ø> (ø)
snuba/reader.py	91.83% <ø> (ø)
snuba/request/schema.py	72.88% <ø> (ø)
snuba/web/db_query.py	88.50% <ø> (ø)
tests/clusters/fake_cluster.py	86.00% <ø> (ø)
snuba/request/request_settings.py	84.52% <87.50%> (+0.31%)	⬆️
tests/test_snql_api.py	100.00% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b9198f8...e3bd4e7. Read the comment docs.

[fpacifici]

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.