feat: add chart id and dataset id to global logs

[eschutho]

SUMMARY

part two of the context logs, this adds the chart and dataset ids to global context when the /data endpoints are called for use in the sql mutator.

An example of a sql_mutator override to log these values would be:

def mutator(sql: str, **kwargs: Any) -> str:
    key = "6dcd92a04feb50f14bbcf07c661680ba"
    output = (
        f"-- {key}\n"
        + f"-- slice_id: {g.logs_context.get('slice_id') if hasattr(g,'logs_context') else ''}\n"
        + f"-- dataset_id:{g.logs_context.get('dataset_id') if hasattr(g,'logs_context') else ''}\n"
    )
    return output + f"{sql}\n-- {key}"

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

After with the mutator above:

TESTING INSTRUCTIONS

Update the sql mutator override, load a dashboard and click "view query" on a chart.

ADDITIONAL INFORMATION

• Has associated issue:
• Required feature flags:
• Changes UI
• Includes DB Migration (follow approval process in SIP-59)
  • Migration is atomic, supports rollback & is backwards-compatible
  • Confirm DB migration upgrade and downgrade tested
  • Runtime estimates and downtime expectations provided
• Introduces new feature or API
• Removes existing feature or API

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (6a1ce6a) 69.48% compared to head (335752c) 69.49%.
Report is 52 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master   #26443   +/-   ##
=======================================
  Coverage   69.48%   69.49%           
=======================================
  Files        1894     1895    +1     
  Lines       74151    74194   +43     
  Branches     8243     8243           
=======================================
+ Hits        51527    51563   +36     
- Misses      20555    20562    +7     
  Partials     2069     2069           

Flag	Coverage Δ
hive	53.82% <100.00%> (+0.01%)	⬆️
mysql	77.97% <100.00%> (-0.02%)	⬇️
postgres	78.07% <100.00%> (-0.02%)	⬇️
presto	53.77% <100.00%> (+0.01%)	⬆️
python	83.02% <100.00%> (+<0.01%)	⬆️
sqlite	77.65% <100.00%> (-0.02%)	⬇️
unit	56.40% <80.00%> (+<0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[hwmarkcheng]

Would it make sense for this to be a parameter? Right now this is hard coded for query related logging, but it might be useful for other logging purposes.

If we add the available_logs_context_keys as a parameter when the decorator is called we can still keep the functionality of limiting data but for that specific call.

[eschutho]

Yeah, that's a good question. In the SIP review we talked about trying to limit antipatterns of people putting random data into global context so the intention here is to use this list to limit what data can be passed in with the decorator, but there are definitely a lot of other use cases. I have three others on my todo list. I would suggest that if you think there's more data that should be considered for logging, then add it to the list in a PR and we'll see if it fits. That way we're still limiting the data that the decorator can accept, but we're not completely limited to just the set that is here now. How does that sound?

[hwmarkcheng]

Hey Elizabeth, that sounds great! As for now there's no other data we need aside from dataset_id/slice_id so this is good on our end. Was just pointing this out as a future suggestion.

[betodealmeida]

Can we move this to a separate function to increase readability?