Skip to content

[SDBM-1807] Do not submit plan events when plan definition has been evicted from cache #20584

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

Merged
merged 7 commits into from
Jun 30, 2025

Conversation

natashadada
Copy link
Contributor

@natashadada natashadada commented Jun 24, 2025

There are cases where we are not able get a plan from the cache (ex. sqlserver evicted the relevant plan to free up memory). When this happens, we still submit a plan event with a plan signature but no plan definition. When querying for plans this is a bit weird since we say we've collected a plan because we see a plan event with a plan signature but we don't have the actual plan to show the user.

I see a few options here:

  1. We can not submit plan events if we don't have the plan definition. This makes the most sense to me right now but would appreciate other perspectives/things I may be missing
  2. We could submit the plan with a collection_error that says the plan is null because it was not found
  3. We could leave plan collection as is and change the way we query for plans on the front end (only query for plan events with a plan definition)

This PR is for option 1. If there is a collection errors, then we still submit the plan but if there's not collection error and no plan definition, we skip the plan event. We have a log line when we fail to load plan, so users can still see when this happens and we can see it in debug flares.

If a plan is encrypted, we don't get a plan definition. In this case, I am adding a collection error. I will update the front end to have support for this error in https://github.com/DataDog/web-ui/pull/212997.

Support ticket: https://datadoghq.atlassian.net/browse/SDBM-1807

@natashadada natashadada force-pushed the natasha.dada/SDBM-1807 branch from 0ccce83 to a09cfe0 Compare June 25, 2025 17:34
Copy link

codecov bot commented Jun 25, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.38%. Comparing base (077bcac) to head (42def46).
Report is 13 commits behind head on master.

Additional details and impacted files
Flag Coverage Δ
activemq ?
cassandra ?
confluent_platform ?
hive ?
hivemq ?
hudi ?
ignite ?
jboss_wildfly ?
kafka ?
presto ?
solr ?
sqlserver 91.27% <100.00%> (+5.30%) ⬆️
tomcat ?
weblogic ?

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@natashadada natashadada changed the title [SDBM-1807] Only submit plan events with plan definitions [SDBM-1807] Do not submit plan events when plan definition has been evicted from cache Jun 26, 2025
@natashadada natashadada added this pull request to the merge queue Jun 30, 2025
Merged via the queue into master with commit 80c1637 Jun 30, 2025
24 checks passed
@natashadada natashadada deleted the natasha.dada/SDBM-1807 branch June 30, 2025 14:54
github-actions bot pushed a commit that referenced this pull request Jun 30, 2025
…victed from cache (#20584)

* [SDBM-1807] Only submit plan events with plan definitions

* lint

* fix

* lint

* lint

* fix

* update test 80c1637
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants