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

[7.17] Some CSV reporting tests fail when run against ES 8.0 #123082

Closed
lukeelmers opened this issue Jan 14, 2022 · 8 comments
Closed

[7.17] Some CSV reporting tests fail when run against ES 8.0 #123082

lukeelmers opened this issue Jan 14, 2022 · 8 comments
Assignees
Labels
bug Fixes for quality problems that affect the customer experience (Deprecated) Feature:Reporting Use Reporting:Screenshot, Reporting:CSV, or Reporting:Framework instead Feature:Reporting:CSV Reporting issues pertaining to CSV file export impact:high Addressing this issue will have a high level of impact on the quality/strength of our product. loe:small Small Level of Effort needs-team Issues missing a team label v7.17.0

Comments

@lukeelmers
Copy link
Member

lukeelmers commented Jan 14, 2022

When testing 7.17 Kibana against 8.0 Elasticsearch, we get some CSV reporting-related failures from CI:

[job] [logs] Default CI Group #25 / discover Discover CSV Export Generate CSV: new search generates a report from a new search with data: default
[job] [logs] Default CI Group #25 / discover Discover CSV Export Generate CSV: new search generates a report from a new search with data: default
[job] [logs] Default CI Group #2 / Reporting APIs CSV Generation from SearchSource Exports CSV with almost all fields when using fieldsFromSource
[job] [logs] Default CI Group #2 / Reporting APIs CSV Generation from SearchSource Exports CSV with almost all fields when using fieldsFromSource

https://buildkite.com/elastic/kibana-7-dot-latest-es-8-dot-0-forward-compatibility/builds/15

In 7.17, basic Kibana functions are still expected to work when run against ES 8.0, because during an upgrade process users will upgrade ES first.

So the ask here is to:

  • investigate the failures from the build
  • determine if they are simply bugs in our tests, or legit issues
  • if they are legit issues, we should determine if they need to be raised as 7.17 blockers

To test locally (be sure to update the config paths & string to grep for):

git checkout 7.17
TEST_ES_BRANCH=8.0.0 node scripts/functional_tests_server.js --config=path/to/suite/config.ts
# in another terminal session
node scripts/functional_test_runner --config=path/to/suite/config.ts --grep="whatever"
@lukeelmers lukeelmers added bug Fixes for quality problems that affect the customer experience (Deprecated) Feature:Reporting Use Reporting:Screenshot, Reporting:CSV, or Reporting:Framework instead Team:AppServicesSv v7.17.0 labels Jan 14, 2022
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-reporting-services (Team:Reporting Services)

@tsullivan tsullivan self-assigned this Jan 14, 2022
@tsullivan
Copy link
Member

@lukeelmers thank you for including the commands to run the test locally!

I am guessing these Reporting CSV tests are failing due to a problem with the test themselves. They error out as soon as I try to load the es archive: https://github.com/elastic/kibana/blob/no-include_type_name/x-pack/test/reporting_api_integration/services/scenarios.ts#L54

That triggers this rejection:

ResponseError: illegal_argument_exception: [illegal_argument_exception] Reason: request [/ecommerce] contains unrecognized parameter: [include_type_name]

Is there an alternative way to load the archive data?

@rudolf
Copy link
Contributor

rudolf commented Jan 19, 2022

@tsullivan Was this fixed by #123093 or is further work required?

@tsullivan
Copy link
Member

Hi @rudolf,
Mikhail reverted the PR as it causes the tests to fail when run against 7.x ES.

These tests are specific to 7.x. The tests blindly query for all of the fields that can be returned from ES. In 7.x, the results include _type and in 8.0, they do not.

This doesn't seem like a blocker to me. The test coverage for CSV export with ES 8.0 is adequate in the main and 8.0 branches. We also have working tests when 7.17 is run against 8.0, where the request to generate CSV includes a set list of fields, and there is no _type interplay.

Could we flag these 7.17 tests in a way that will skip them when the test server is using ES 8.0?

@exalate-issue-sync exalate-issue-sync bot added impact:needs-assessment Product and/or Engineering needs to evaluate the impact of the change. loe:small Small Level of Effort labels Jan 24, 2022
@tsullivan
Copy link
Member

Looks like these tests will be skipped in this PR: #123312

It will add this.onlyEsVersion('<=7') to our tests in 7.17 branch only. This will be the best outcome for the tests in 7.17: the work to adapt them for cross-version is going to be more work than it's worth. And, there is already a lot of test coverage on other csv exports.

@tsullivan
Copy link
Member

I'm not sure the tests were skipped as selectively as they should have been: https://github.com/elastic/dev/issues/1855#issuecomment-1021444827

@tsullivan tsullivan reopened this Jan 25, 2022
@tsullivan
Copy link
Member

tsullivan commented Jan 26, 2022

@jloleysens @dokmic could one of you take a look into this? In the 7.17 branch, we have this.onlyEsVersion('<=7') statements in the code for CSV tests:

> x-pack/test/functional/apps/discover/reporting.ts:    this.onlyEsVersion('<=7');
  x-pack/test/reporting_api_integration/reporting_and_security/ilm_migration_apis.ts:    this.onlyEsVersion('<=7');
  x-pack/test/reporting_api_integration/reporting_and_security/generate_csv_discover.ts:    this.onlyEsVersion('<=7');
  x-pack/test/reporting_api_integration/reporting_and_security/search_frozen_indices.ts:    this.onlyEsVersion('<=7');
  x-pack/test/reporting_api_integration/reporting_and_security/download_csv_dashboard.ts:    this.onlyEsVersion('<=7');

We should make sure these statements are not skipping more tests than is necessary. The only tests that make sense to skip when Kibana 7.17 is run against ES 8.0 are the ones that export "all" the fields from the index. In those tests, ES in 7.x will return a _type field, but in 8.0 it will not.

@exalate-issue-sync exalate-issue-sync bot added impact:high Addressing this issue will have a high level of impact on the quality/strength of our product. and removed impact:needs-assessment Product and/or Engineering needs to evaluate the impact of the change. labels Feb 1, 2022
@tsullivan
Copy link
Member

Resolved by #123981

@exalate-issue-sync exalate-issue-sync bot reopened this Feb 3, 2022
@sophiec20 sophiec20 added the Feature:Reporting:CSV Reporting issues pertaining to CSV file export label Aug 21, 2024
@botelastic botelastic bot added the needs-team Issues missing a team label label Aug 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Fixes for quality problems that affect the customer experience (Deprecated) Feature:Reporting Use Reporting:Screenshot, Reporting:CSV, or Reporting:Framework instead Feature:Reporting:CSV Reporting issues pertaining to CSV file export impact:high Addressing this issue will have a high level of impact on the quality/strength of our product. loe:small Small Level of Effort needs-team Issues missing a team label v7.17.0
Projects
None yet
Development

No branches or pull requests

5 participants