bug(compass): create_time and update_time are not being sent

[StewartJingga]

Is your feature request related to a problem? Please describe. Our current Asset model has below fields to store metadata create & update time.

// The timestamp when the object was created.
CreateTime *timestamppb.Timestamp `json:"create_time,omitempty"`

// The timestamp when the object was last modified.
UpdateTime *timestamppb.Timestamp `json:"update_time,omitempty"`

Which will be populated by extractor when's possible.

But when sinking to compass, these CreateTime and UpdateTime are not being sent.

This is because Compass request payload does not have any fields to accomodate this except Data field which is basically just a map[string]interface{}. And when sending to Compass, Meteor sets Data field directly from Asset.Data which is a concrete model that does not have those time fields as well.

Describe the solution you'd like

1. Move time fields from Asset to Table, Dashboard, Topic, etc.

This will modify Meteor's Table | Dashboard | Topic | ... models to have create_time and update_time fields while removing it from Asset.

When sinking to compass, these create_time and update_time will be sent along inside Data field in compass payload.

2. Open proposal in odpf/compass to have those time fields in its Asset model.

If compass.Asset model has these fields available, then Meteor would just need to pass those time fields directly from its Asset.

compassAsset := Asset{
  ...other fields
  URN: meteorAsset.GetUrn(),
  CreateTime: meteorAsset.CreateTime,
  UpdateTime: meteorAsset.UpdateTime,
}

[ravisuhag]

@StewartJingga I think the second approach is better.

[StewartJingga]

@ravisuhag ok so in Compass.Asset, it would be like below

type Asset struct {
  ...other fields
  URN         string
  CreateTime  time.Time              
  UpdateTime  time.Time              
  CreatedAt   time.Time              
  UpdatedAt   time.Time
}

will that be confusing?

[ravisuhag]

Yes, this makes it confusing for sure. One way could be to rename compass related timestamp to ingestedAt etc. Or you think first approach is better? The only problem i see with first is that timestamp will be duplicated in all models.

[StewartJingga]

Yes, this makes it confusing for sure. One way could be to rename compass related timestamp to ingestedAt etc. Or you think first approach is better? The only problem i see with first is that timestamp will be duplicated in all models.

@ravisuhag Right now, CreatedAt and UpdatedAt is Compass' internal time values, changing it have some challenges as well such as handling backward compatibility.

We could probably rename CreateTime and UpdateTime to make it clearer that they are representing metadata's times and not Compass internal times. Example would be MetadataCreateTime, MetadataUpdateTime.

Or we can use the first approach, which as you mentioned, also has its downside (duplication).

[sudo-suhas]

One more reason to rename those fields is the asset response for GetAllAssets/GetAssetByID etc. The asset in the response for these includes the created_at and updated_at fields sourced from the internal time values present in Compass - odpf/proton@v0.2.0/odpf/compass/v1beta1/service.proto#L1253-L1254. So we would need to have distinct names in the response as well.

[sudo-suhas]

To keep it intuitive from the user perspective, we will be going with the first proposed approach and adding the fields into Meteor's Table, Dashboard, Topic etc. The intent is to keep it clear that Asset.{CreatedAt, UpdatedAt} are related to ingestion and Asset.Data.{CreateTime, UpdateTime} are the timestamps of the resource in the external system.