SY-3039: Fix ontology relationships of node metrics channels #1828
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
…9-fix-ontology-relationship-with-node-metrics-channels
core/pkg/service/metrics/service.go
Outdated
| ctx, | ||
| &c.idx, | ||
| channel.RetrieveIfNameExists(), | ||
| channel.CreateWithoutGroupRelationship(), |
There was a problem hiding this comment.
Is there a reason you chose this API over a CreateWithParent style API like we have in other services? Overall ok with this but we need to decide on a consistent strategy at some point for how we decide on ontology parents
There was a problem hiding this comment.
It felt like it fit a little bit more with how the creation of channels is already done
with the create options. I'm not married to this API but I thought it was the simplest
to get in and not worth overthinking until we put more effort to standardizing how the
ontology is used throughout all services.
core/pkg/service/metrics/service.go
Outdated
| // delete any existing relationships between the parent Channels group and the | ||
| // metrics channels | ||
| for _, ch := range metricChannels { | ||
| if err := cfg.Ontology.NewWriter(nil).DeleteRelationship(ctx, cfg.Channel.Group().OntologyID(), ontology.ParentOf, ch.OntologyID()); err != nil { |
There was a problem hiding this comment.
Would I be right in thinking that This overflows 88 chars?
| close(s.stopCollector) | ||
| return s.shutdown.Close() | ||
| } | ||
| func (s *Service) Close() error { close(s.stopCollector); return s.shutdown.Close() } |
There was a problem hiding this comment.
I thought you didn't like these?
There was a problem hiding this comment.
I changed my mind and am good on these as long as they don't overflow the 88 character limit
Issue Pull Request
Linear Issue
SY-3039
Description
Fix ontology relationships with node metrics channels by creating a metrics and a node group to group all channels within.
Basic Readiness
Greptile Overview
Greptile Summary
This PR fixes the ontology relationships for node metrics channels by introducing a proper hierarchical structure. Previously, metrics channels were created as direct children of the root "Channels" group, making them difficult to organize and navigate.
Key Changes
Ontology Hierarchy: Introduces a two-level group structure:
Channels→Metrics→Node {hostKey}→ [individual metric channels]This provides better organization by grouping all metrics together, then separating them by node.
Channel Creation Control: Adds
CreateWithoutGroupRelationshipoption to channel creation, allowing channels to be created without automatically linking to the default Channels group. This is essential for metrics channels that need custom parent relationships.Relationship Cleanup: After establishing the new hierarchy, the code explicitly removes any existing relationships between the Channels group and metric channels (lines 186-192 in service.go). This handles the migration case where metrics channels may have been previously linked to the Channels group.
Error Handling Improvement: The
group.Service.CreateOrRetrievemethod now properly handles non-NotFound errors usingerrors.Skip, preventing the function from continuing with invalid state.Implementation Quality
The implementation follows established patterns in the codebase:
RetrieveIfNameExistsfor idempotent channel creationCreateOptionsthrough the channel creation stackTesting
Comprehensive test coverage includes:
Confidence Score: 4/5
Important Files Changed
File Analysis
Sequence Diagram
sequenceDiagram participant MS as MetricsService participant GS as GroupService participant CS as ChannelService participant OS as OntologyService MS->>GS: CreateOrRetrieve("Metrics", ChannelsGroupID) GS->>GS: Retrieve existing or create new GS-->>MS: metricsGroup MS->>GS: CreateOrRetrieve("Node {hostKey}", metricsGroupID) GS->>GS: Retrieve existing or create new GS-->>MS: nodeGroup MS->>CS: Create(indexChannel, CreateWithoutGroupRelationship) CS->>CS: Skip default group linking CS-->>MS: Index channel created MS->>OS: DefineRelationship(nodeGroup, ParentOf, indexChannel) OS-->>MS: Relationship created MS->>CS: CreateMany(metricChannels, CreateWithoutGroupRelationship) CS->>CS: Skip default group linking for all CS-->>MS: Metric channels created MS->>OS: DefineFromOneToManyRelationships(nodeGroup, ParentOf, metricChannels) OS-->>MS: Relationships created loop For each metric channel MS->>OS: DeleteRelationship(ChannelsGroup, ParentOf, metricChannel) OS-->>MS: Old relationship removed end Note over MS,OS: Hierarchy: Channels → Metrics → Node {hostKey} → [metric channels]