[Infra UI] Add APM Section to Node Detail Page #42710
Conversation
- Adds APM feature to metadata - Closes elastic#42167 - Based on elastic#41836
- Closes elastic#42169 - Adds /api/infra/apm_metrics - Adds makeInternalRequest method to Framwork library - Adds scafolding for tests - Removes duplicate version of Boom - Adds tests
simianhacker
added
Feature:Metrics UI
|
Pinging @elastic/infra-logs-ui |
💔 Build Failed |
simianhacker
force-pushed
the
fixes-42170
branch
from
4079285 to
27bf1d4
Compare
💔 Build Failed |
- Closes elastic#42170 - Adds useApmMetrics hook - Adds APM layout - Adds APM section to node detail page
simianhacker
force-pushed
the
fixes-42170
branch
from
27bf1d4 to
c7c54e4
Compare
💔 Build Failed |
💔 Build Failed |
💔 Build Failed |
💔 Build Failed |
💚 Build Succeeded |
|
@sorantis @jasonrhodes I would feel more comfortable if both of you signed off on this one. If you both think we should wait to add this later then I can close it and re-open as a draft. |
💚 Build Succeeded |
There was a problem hiding this comment.
@simianhacker TBH I wasn't following the discussion around the APM section, but from what I could gather, the section is in good shape and can be merged.
BTW, #43787 is a great feature!
|
@sorantis Thanks! I was testing some stuff today and notice that individual points now show up where before they would have been totally overlooked. Having the points and legends turned off looks slick but at the expense of usefulness. |
💔 Build Failed |
💚 Build Succeeded |
💚 Build Succeeded |
|
@jasonrhodes Before this can be reviewed I need to refactor it so it's not using the internal request API. |
💚 Build Succeeded |
💔 Build Failed |
💚 Build Succeeded |
💚 Build Succeeded |
There was a problem hiding this comment.
I spoke with @simianhacker about this PR in detail last week and spent time looking it over. I think I agree with Chris's overall personal sentiment he expressed to me that this is a lot of changes (more than we expected at first) and a lot of it may need to be redone for the new platform, so I think it may be better to close this and reference it again later in chunks.
Two questions:
-
@elastic/apm-ui can you look at the changes here that Chris made to your files and see if you are okay with them? I'd like to explore landing those changes soon and see how exposing that kind of JS API works for us as shared observability teams, as well as making it something that gets migrated to the NP as well...
-
@sorantis do we have a sense of how much appetite there is for this particular integration? If we have the impression people are wanting this badly, we could still work to land it for 7.5, but I'm thinking the focus it may pull from the AWS inventory stuff may not be worth it. But I don't know what the product priorities are, so I'd like to have product weigh in there.
|
Based on the comments from @jasonrhodes I'm going to close this PR. We can revisit this after there has been wider adoption of the New Platform and more consideration around sharing access to data. |
Summary
This PR add the APM data endpoint along with the UI for adding APM to the node detail page. This PR also includes the APM Metrics API which was originally going to be a separate PR but I decided to include it in this PR since some of the assumptions change while adding the UI elements.
I added a new
InfraNodeTypetocommon/http_api/common.tsthat will eventually replace the enumInfraNodeTypefrom GraphQL. I'm planning on creating a separate PR after this to remove the rest of GraphQL from the Node Detail page. Once the node detail page is converted away from GraphQL, the apm_metrics endpoint should be consolidated into the same endpoint as the reset of the metrics to remove some of the complexities inwithMetricsCloses #41005
Closes #42170
Closes #42169
Checklist
Use
strikethroughsto remove checklist items you don't feel are applicable to this PR.Documentation was added for features that require explanation or tutorials