Kibana: [APM] Don't read APM index names directly from config

Created on 29 Oct 2019  路  14Comments  路  Source: elastic/kibana

In https://github.com/elastic/kibana/pull/48079 APM added support for overriding index names defined in kibana.yml via the UI. This means that reading a config value directly from the config might give a stale result, since it's being overridden by the UI settings.

Instead of reading directly from the config file one should read both from the config file and the UI settings and merge the result. Fortunately this is already taken care of by getApmIndices.

Example: apm_oss.transactionIndices
Instead of getting apm_oss.transactionIndices via config.get('apm_oss.transactionIndices') one should instead get it like:

import { getApmIndices } from 'x-pack/legacy/plugins/apm/server/lib/settings/apm_indices/get_apm_indices'

const apmIndices = getApmIndices(server)
const transactionIndices = apmIndices['apm_oss.transactionIndices']

Fix in

  • [x] [x-pack/legacy/plugins/infra/server/routes/metadata/lib/has_apm_data.ts](https://github.com/elastic/kibana/blob/5e05bbd05fa8f4bee67afeb42bccb44b38910534/x-pack/legacy/plugins/infra/server/routes/metadata/lib/has_apm_data.ts#L22)
  • [ ] [src/legacy/core_plugins/kibana/server/tutorials/apm/envs/on_prem.js](https://github.com/elastic/kibana/blob/a6d0b6018bcd9e1c7688465e91c0934d5f9cd422/src/legacy/core_plugins/kibana/server/tutorials/apm/envs/on_prem.js#L173-L178)
apm logs-metrics-ui v7.6.0

All 14 comments

Pinging @elastic/apm-ui (Team:apm)

Pinging @elastic/logs-metrics-ui (Team:logs-metrics-ui)

@elastic/apm-ui hey thanks for this heads up! I'm trying to use this in our NP shim layer now, where we will no longer have access to the server object. Can we talk through how we could make that possible?

@jasonrhodes After https://github.com/elastic/kibana/pull/49455 lands, server will no longer be required, but as it stands, getApmIndices would still need the APM configuration object. Not sure what the right solution is, the infra plugin could make APM an optional dependency, or we could expose an API endpoint.

@dgieselaar I think the right solution will be for APM to return "getIndices" from its setup method, so that we can just call that directly. That is the place for plugins to share a contract, with types, as I understand it.

I'm going to shim that method in a fake APM plugin for now, to get it working, and we can hammer out the details. :D

Also once #49994 merges, the server param will change to a single object param: { config: KibanaConfig, savedObjectsClient: ScopedSavedObjectsClient } https://github.com/elastic/kibana/pull/49994/files#diff-c8001ab8253c76efb10e2b90d98e2e36R60-R63. So It might be best to hold off until both #49455 and #49994 are merged.

Labelling as blocked since #49455 is blocked.

I've removed the blocked label from #49455, and will start work on this based off of that branch.

I'm not 100% sure whether we can fix the tutorial. It's all synchronous stuff over there. I mean, we _can_ fix it by changing all the registerTutorial stuff to support async, but is it worth it at that point? I would imagine people would follow any tutorial before they start configuring UI indices.

I would imagine people would follow any tutorial before they start configuring UI indices

Yeah, I'm okay calling it an edge case. Additionally the tutorials might be revamped in the not so distant future so might be wasted work if we do it now.

I'd like to scope this ticket to exposing an getAPMIndices() method from the NP APM plugin contract, and then leave it up to infra to use it, WDYT @sqren? Looks like it requires quite a bit of plumbing from the infra side, e.g. a dependency on the APM NP plugin in a legacy infra plugin, and passing the APM contract (or part of it) through a few layers until we're at hasAPMData.

@dgieselaar SGTM 馃憤

Was this page helpful?
0 / 5 - 0 ratings

Related issues

Ginja picture Ginja  路  3Comments

bradvido picture bradvido  路  3Comments

cafuego picture cafuego  路  3Comments

timmolter picture timmolter  路  3Comments

LukeMathWalker picture LukeMathWalker  路  3Comments