[Maps] Fix regression preventing maps telemetry from populating & remove task manager logic

[kindsun]

Resolves #53809. Following up on offline discussion started as a result of #52439. Changes made in this PR:

• The Maps app originally required the task manager to harvest data from Elasticsearch 1x/day. Telemetry would then in turn pull data from the task manager's state. This functionality hasn't been needed since Monitoring telemetry collection interval #34609. Task manager and any associated tests have been removed. Telemetry is now harvested directly from Elasticsearch 1x/day or when the stats API endpoint is directly queried.
• Fixed an issue that prevented Maps telemetry from updating in 7.5.0-1
• Added maps to telemetry integration tests

Testing this PR:

• Load sample data
• Go the stats API endpoint (http://localhost:5601/<dev url prefix>/api/stats?extended=true). It usually takes a minute or 2 after starting Kibana for this to be available. The result should look something like this:

"maps_telemetry":{
  "type":"maps-telemetry",
  "id":"maps-telemetry",
  "attributes":{
    "settings":{
      "show_map_visualization_types":false
    },
    "index_patterns_with_geo_field_count":3,
    "maps_total_count":3,
    "time_captured":"2019-12-23T20:25:17.421Z",
    "attributes_per_map":{
      "data_sources_count":{
        "min":4,
        "max":7,
        "avg":5
      },
      "layers_count":{
        "min":4,
        "max":7,
        "avg":5
      },
      "layer_types_count":{
        "vector_tile":{
          "min":1,
          "max":1,
          "avg":1
        },
        "vector":{
          "min":3,
          "max":6,
          "avg":4
        }
      },
      "ems_vector_layers_count":{
        "world_countries":{
          "min":1,
          "max":1,
          "avg":0.6666666666666666
        },
        "usa_states":{
          "min":1,
          "max":1,
          "avg":0.3333333333333333
        },
        "france_departments":{
          "min":1,
          "max":1,
          "avg":0.3333333333333333
        },
        "uk_subdivisions":{
          "min":1,
          "max":1,
          "avg":0.3333333333333333
        }
      }
    }
  },
  "references":[

  ],
  "updated_at":"2019-12-23T20:25:17.422Z",
  "version":"WzY1LDFd"
},

This PR was also tested to ensure telemetry wasn't querying Elasticsearch more than expected with the removal of task manager since it used to be every 10 seconds. To confirm:

• Set a node breakpoint (or log output) in maps_telemetry.js > getMapsTelemetry()
• Note the function isn't called unless the stats endpoint listed above is loaded/refreshed.

[elasticmachine]

Pinging @elastic/kibana-gis (Team:Geo)

[Bamieh]

LGTM! it seems that the api integration test is failing. Since the data is pulled from a saved object maybe you'd need to make sure that the saved object is being populated first in the test (populate the saved object via api)

[kindsun]

LGTM! it seems that the api integration test is failing. Since the data is pulled from a saved object maybe you'd need to make sure that the saved object is being populated first in the test (populate the saved object via api)

@Bamieh Thanks! Seems like it was referencing "maps" instead of "maps-telemetry" as it should've (oops). We'll see shortly!

[thomasneirynck]

After discussion with @aaronjcaldwell, will investigate if the fix for the regression can be extracted in a separate PR.

This PR includes np-changes that can't be backported to a 7.5 patch release and will cause merge conflicts on the 7.5 branch. These merge conflicts would require fixing the regression without the np-changes anyway for 7.5. So instead of having to do this ad-hoc fix as the result of a failed backport, it might be cleaner to split this PR in two separate PRs ((1) fix regression->backport to 7.5/7.x and (2) np-changes ->backport to 7.x).

[thomasneirynck]

After discussion, this PR is blocked by #53803

[kindsun]

Is this a PR an isolated fix for #53809?

There were a number of other improvements I snuck in initially, but after some offline discussion I've backed those out and will handle in a separate PR. The changes in this PR are specifically aimed at removing the Task Manager logic, which imho is the correct solution here. The original issues stemmed from async task initialization, an alternative fix was proposed here. While a version of that fix might work, it still leaves the app vulnerable to task manager issues without conferring any of task manager's original benefit for maps (i.e.- the ability to leverage task manager state which we no longer need). Basically, this PR fixes the regression by removing the thing that broke (task initialization) and greatly simplifying Maps telemetry to be inline with the majority of other Kibana apps without compromising any of its capabilities.

[kibanamachine]

💚 Build Succeeded

• continuous-integration/kibana-ci/pull-request
• Commit: 422b363

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

[kindsun]

@elasticmachine merge upstream

[thomasneirynck]

So this looks really great, in the sense that it removes a lot of code.

But I am having trouble to evaluate this, so it might help if somebody with more telemetry knowledge can review this as well.

Also, for the backport to 7.5, let's not merge this into 7.5 before that backport-PR has been reviewed explicitly against the 7.5 branch, given that it introduces a new architecture for collecting the telemetry.

[kindsun]

@Bamieh Would you mind giving this one another pass? Since we've both removed task manager and made some telemetry changes, would be great to get your eyes on it again!

[Bamieh]

code changes LGTM!

[kibanamachine]

💚 Build Succeeded

• continuous-integration/kibana-ci/pull-request
• Commit: 97730c8

History

• 💔 Build #18126 failed 1ab2bfc
• 💚 Build #17438 succeeded a2f3e6d

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream