Skip to content

Log the stacktrace when we encounter a deprecation warning for default_metric#143929

Merged
gmarouli merged 3 commits intoelastic:mainfrom
gmarouli:mitigate-unexpected-warning-header-in-flaky-tests
Mar 10, 2026
Merged

Log the stacktrace when we encounter a deprecation warning for default_metric#143929
gmarouli merged 3 commits intoelastic:mainfrom
gmarouli:mitigate-unexpected-warning-header-in-flaky-tests

Conversation

@gmarouli
Copy link
Copy Markdown
Contributor

@gmarouli gmarouli commented Mar 10, 2026

While we investigate #143884, we enhance log the stack trace during debug mode but we also allow the unexpected warning in our ci, so the tests will not remain muted for too long.

When #143884 is closed, this PR needs to be reverted (apart from the test unmuting).

@gmarouli gmarouli added >test Issues or PRs that are addressing/adding tests :Core/Infra/Core Core issues without another label :StorageEngine/Mapping The storage related side of mappings labels Mar 10, 2026
@elasticsearchmachine
Copy link
Copy Markdown
Collaborator

Pinging @elastic/es-storage-engine (Team:StorageEngine)

@elasticsearchmachine
Copy link
Copy Markdown
Collaborator

Pinging @elastic/es-core-infra (Team:Core/Infra)

Comment on lines +693 to +694
cluster.put_settings:
body: >
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Also, enabling the logger here for this one test case is likely to miss the source of the leak, which we suspect to be an earlier test case. Let's turn on this logging for the whole cluster for the duration of the test.

@gmarouli gmarouli requested a review from DaveCTurner March 10, 2026 09:34
} catch (WarningFailureException warningException) {
Map<String, Object> indexMapping = ESRestTestCase.getIndexMapping("metrics-long");
logger.error("Received warning when creating index [metrics-long] with mapping [{}]", indexMapping);
// Remove try-catch after https://github.com/elastic/elasticsearch/issues/143884
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What try-catch? 😁

logger.error("Received warning when creating index [metrics-long] with mapping [{}]", indexMapping);
// Remove try-catch after https://github.com/elastic/elasticsearch/issues/143884
List<String> warnings = warningException.getResponse().getWarnings();
if (warnings.size() == 1
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why not if multiple warnings?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Ah, I was too sloppy 😮‍💨

setup:
- requires:
test_runner_features: [allowed_warnings_regex, allowed_warnings]
# Remove enabling debug logging after https://github.com/elastic/elasticsearch/issues/143884
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This one too - if the warning is leaking in from an earlier test then we won't catch it like this. We need the logging enabled for the whole test run.

@gmarouli gmarouli changed the title Accept unexpected warning and log the stacktrace Log the stacktrace when we encounter a deprecation warning for default_metric Mar 10, 2026
Copy link
Copy Markdown
Member

@DaveCTurner DaveCTurner left a comment

Choose a reason for hiding this comment

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

LGTM

@gmarouli
Copy link
Copy Markdown
Contributor Author

LGTM

Thank you for the quick and sharp review @DaveCTurner .

@gmarouli gmarouli merged commit f00793a into elastic:main Mar 10, 2026
35 of 36 checks passed
@gmarouli gmarouli deleted the mitigate-unexpected-warning-header-in-flaky-tests branch March 10, 2026 11:58
szybia added a commit to szybia/elasticsearch that referenced this pull request Mar 10, 2026
…locations

* upstream/main: (126 commits)
  Update KnnIndexTester to use more settings from datasets (elastic#143869)
  fix: dynamic template vector array is overridden by automatic dense_vector mapping (elastic#143733)
  ES|QL: Don't reuse the same alias for _fork column (elastic#143909)
  Close and initialize clients after each node upgrade in logsdb rolling upgrade tests. (elastic#143823)
  ESQL: Added GroupedTopNOperator for LIMIT BY, compute only (elastic#143476)
  Handle views in ResolveIndexAction (elastic#143561)
  Improve reindex rethrottle API in stateless (elastic#143771)
  Use a copy of the SearchExecutionContext for each Percolator execution (elastic#142765)
  Log the stacktrace when we encounter a deprecation warning for `default_metric` (elastic#143929)
  ESQL: evaluate ReferenceAttributes to potentially FieldAttributes for full-text functions restriction (elastic#143893)
  Add ClusterStateSerializationStats Serializatation Tests (elastic#142703)
  Adds Coordination Diagnostics Tests (elastic#142709)
  Upgrade Elasticsearch to Apache Lucene 10.4 (elastic#141882)
  ESQL: Add configurable bracket-based multi-value support for CSV reader (elastic#143890)
  time series es819 binary dv use up to a 1mb block size (elastic#143049)
  Dynamically enable / disable plugins in correspondence to stateless mode. (elastic#142147)
  ES|QL: Implement first/last_over_time for tdigest (elastic#143832)
  Document CHANGE_POINT limitation (elastic#143877)
  Fix OperationsOnSeqNoDisabledIndicesIT (elastic#143892)
  [Test] Test that sequence numbers are not pruned with retention lease (elastic#143825)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Core/Infra/Core Core issues without another label :StorageEngine/Mapping The storage related side of mappings Team:Core/Infra Meta label for core/infra team Team:StorageEngine >test Issues or PRs that are addressing/adding tests v9.4.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants