[APM] Fix missing ML data, and NaN issue #34333
Conversation
💔 Build Failed |
| console.log({ apmIndices, indexParam }); | ||
| if (isString(indexParam)) { | ||
| return apmIndices.includes(indexParam); | ||
| } else if (Array.isArray(indexParam)) { |
There was a problem hiding this comment.
When is this an array? And can it be an array of mixed (some apm and some non-apm)? Just wondering if that would end up breaking the UI with 6.x apm data leaking in…
There was a problem hiding this comment.
When is this an array?
It's an array when we want to query multiple indices, like here:
kibana/x-pack/plugins/apm/server/lib/services/get_service.ts
Lines 42 to 45 in d3dd95d
And can it be an array of mixed (some apm and some non-apm)?
We don't do that atm, and I don't think we will. That being said, I added a test for this exact scenario, and if any of the indices are not apm indices we will not add the filter. In that case 6.x data will leak in, but I think we'll have to cross that bridge when we get there. I don't know how else to do this, so unless you have another solution that takes that into account, I think we can leave it at that.
There was a problem hiding this comment.
Yeah, the only thing I can think is to throw an error here if the indices are mixed, to alert us on the first time we try to do that so we remember this caveat. But that may be overkill if we expect this problem to sort of disappear as we get further and further past the 7.0 release...
There was a problem hiding this comment.
Throwing an exception for mixed arrays would add additional logic - I'd prefer to not do that atm.
x-pack/plugins/apm/server/lib/transactions/charts/get_timeseries_data/transform.ts
Show resolved
Hide resolved
💚 Build Succeeded |
sorenlouv
force-pushed
the
fix-nan-and-ml-issue
branch
from
ffd1a9f to
2fd0960
Compare
💔 Build Failed |
|
retest |
1 similar comment
|
retest |
💚 Build Succeeded |
# Conflicts: # x-pack/plugins/apm/server/lib/helpers/setup_request.test.ts # x-pack/plugins/apm/server/lib/helpers/setup_request.ts
Closes #27868
Closes #34294