Skip to content
This repository has been archived by the owner on Nov 3, 2023. It is now read-only.

Publish cloud.instance.id ecs field with k8s nodes #154

Merged
merged 1 commit into from
May 9, 2023

Conversation

MichaelKatsoulis
Copy link
Collaborator

Closes #144

@MichaelKatsoulis MichaelKatsoulis marked this pull request as draft May 4, 2023 11:35
internal.WithIndex(assetType, indexNamespace),
)
var assetParents []string
if instanceId != "" {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nits:

do we need the empty parents list at all?

we can make this more DRY as follows:

options := []internal.AssetOption{
	internal.WithAssetTypeAndID(assetType, assetId),
	internal.WithNodeData(o.Name, assetProviderId, &assetStartTime),
	internal.WithIndex(assetType, indexNamespace),
}

if instanceId != "" {
	options = append(options, internal.WithCloudInstanceId(instanceId))
}

internal.Publish(publisher, options...)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice. Thanks! Regarding the parents of the nodes I guess it will be always empty list. So we don't need to publish an empty list. Or is it a field that should be always there regardless if it is empty or not? That I don't know

@MichaelKatsoulis MichaelKatsoulis marked this pull request as ready for review May 5, 2023 10:13
@MichaelKatsoulis MichaelKatsoulis requested a review from girodav May 5, 2023 10:13
@MichaelKatsoulis MichaelKatsoulis force-pushed the populate-cloud-instance-id-k8s-node branch from 536ab08 to c40f7e6 Compare May 9, 2023 08:58
@MichaelKatsoulis MichaelKatsoulis added this pull request to the merge queue May 9, 2023
Merged via the queue into main with commit 9f89160 May 9, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Populate cloud.instance.id ECS field from k8s.node asset type
3 participants