feat: add navigation handling for inactive entities #815
Conversation
Codecov Report
@@ Coverage Diff @@
## main #815 +/- ##
==========================================
- Coverage 85.44% 85.44% -0.01%
==========================================
Files 792 792
Lines 16199 16212 +13
Branches 2071 2073 +2
==========================================
+ Hits 13841 13852 +11
- Misses 2327 2329 +2
Partials 31 31
Continue to review full report at Codecov.
|
This comment has been minimized.
This comment has been minimized.
projects/observability/src/shared/components/table/data-cell/entity/entity-table-cell-parser.ts
Show resolved
Hide resolved
...ability/src/shared/components/table/data-cell/entity/entity-table-cell-renderer.component.ts
Show resolved
Hide resolved
| @@ -27,3 +30,22 @@ export const parseEntityFromTableRow = (cell: Entity | undefined, row: Dictionar | |||
| }; | |||
|
|
|||
| const isInteraction = (neighbor: unknown): neighbor is Interaction => typeof neighbor === 'object'; | |||
|
|
|||
| export const isInactiveEntity = (row: TableRow): boolean | undefined => { | |||
| const maxEndTimeAggregation = row['max(endTime)']; // Ew. | |||
There was a problem hiding this comment.
This seems... ripe for improvement. Unless we change the api to pass this info in (which wouldn't be a bad idea but likely won't happen immediately), a better heuristic for an inactive entity that avoids looking at any particular field which may or may not be requested/exist on all entities would be to check:
- The entity includes at least one metric AND
- All metrics fetched are undefined
There was a problem hiding this comment.
Yeah, this is really really ugly. I felt better putting it in this file which already holds some ugly Entity handling code. At least it is all isolated.
Talked with the backend folks about which heuristic to use here. Checking that max(endTime) is a valid number is the most solid way to know for sure.
There was a problem hiding this comment.
Talked with the backend folks about which heuristic to use here. Checking that max(endTime) is a valid number is the most solid way to know for sure.
See above - max(endTime) is a specific subset of the general heuristic I described which allows us to remove any coupling to a specific usage. No metric will be available for an inactive entity, so it just limits us to pick one to check.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
...bservability/src/shared/components/table/data-cell/entity/entity-table-cell-renderer-util.ts
Outdated
Show resolved
Hide resolved
…ntity/entity-table-cell-renderer-util.ts Co-authored-by: Aaron Steinfeld <[email protected]>
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
| public onClickNavigate(): void { | ||
| this.navigable && this.entity && this.entityNavService.navigateToEntity(this.entity); | ||
| this.navigable && this.entity && this.entityNavService.navigateToEntity(this.entity, this.inactive); |
There was a problem hiding this comment.
What does inactive do? why do we need this?
There was a problem hiding this comment.
If an entity is inactive, it means we haven't seen any spans in the time range. Therefore, some of the screens won't show any data available and we may want to navigate to alternative pages instead for those APIs when drilled in to.
There was a problem hiding this comment.
So is it same as under discovery entity?
The name is not clear honestly. navigationFunction(entityId, sourceRoute, isInactive) i guess this alternate url would be defined in the entity map?
Description
This change allows navigation routes to know when an entity is inactive so it can be routed differently if needed.