[audit] Figure out timestamps in event objects

[crassirostris]

Ref #48561

Currently, there's a Timestamp field in the audit.Event object, which represents Time the request reached the apiserver.. There're several problems with it:

1. Since auditing process has several stages, each one generating a separate event object in the sink, name can be confusing: a person can understand it as a timestamp of the last auditing process stage.
2. There's another field in the Event, inherited from the ObjectMeta: CreationTimestamps. It's also confusing what it means: either the same what Timestamp currently means or the stage timestmap.
3. CreationTimestamp has seconds precision.
4. Currently CreationTimestamp is not populated and left as nil in the serialized object.
5. Currently there's no stage timestamp in the object, so it's not possible to understand from the last audit object, how much time the request took: you have to correlate events by the AuditId (which can be spoofed).

We agreed that the @soltysh's PR (#52030) will addressed the latter two points by populating the CreationTimestamp field with the stage timestamp, exposing this information and making CreationTimestamp field useful. However, that doesn't solve the problem of ambiguous names or CreationTimestamp precision. There are three options:

1. Leave it as it is.
2. Rename Timestamp field e.g. to RequestStartTimestamp
3. Remove Timestamp field, introduce two new fields RequestStartTimestamp and StageTimestamp and remove ObjectMeta. This is the only option that will allow to use timestamps to make an accurate estimation of how much time the request took, since CreationTimestamp has only seconds precision.

Renaming/removing field ofc will happen in the backward-compatible way, by creating a new field that will hold the same value and then actually removing the field in the next API release.

Because of the precision problem, the last options seems like the best approach.

/cc @sttts @soltysh @ericchiang @CaoShuFeng

[soltysh]

3. Remove Timestamp field, introduce two new fields RequestStartTimestamp and StageTimestamp and remove ObjectMeta (which is possible now, but not tested)

I thought during our discussion we agreed on CreationTimestap being what you call here RequestStartTimestamp (unless we get rid of ObjectMeta if that's possible), introducing a new StageTimestamp (or other name tbd), finally deprecating Timestamp since its name provides no information what kind of time it is.

[sttts]

I thought during our discussion we agreed on CreationTimestap being what you call here RequestStartTimestamp

The opposite. CreationTimestamp is the event object creation time by definition of that field. The Event.Timestamp is the request time (also by definition, i.e. by our godoc).

[sttts]

event object creation time by definition of that field

That we re-use that event object throughout the handler chain is our optimization and an implementation detail. From the outside we create one event per stage.

[CaoShuFeng]

2. Rename Timestamp field e.g. to RequestStartTimestamp

I vote for this option.
But I suggest to rename RequestStartTimestamp to RequestReceivedTimestamp.
And in the RequestReceivedStage:
ObjectMeta.CreationTimestamp is equal to RequestReceivedTimestamp

[crassirostris]

@soltysh Yeah, sorry for confusion earlier, I meant what @sttts said

@CaoShuFeng I also like this option better, b/c removing ObjectMeta will require some testing and even after that something can break, and leaving CreationTimestamp empty doesn't look good

[soltysh]

Important requirement for this is that it should give very good precision, micro- or milisecond precision so we can measure actual time spent for processing requests.

[crassirostris]

Important requirement for this is that it should give very good precision, micro- or milisecond precision so we can measure actual time spent for processing requests.

+1, I'll add this to the issue description

[hzxuzhonghu]

3. Remove Timestamp field, introduce two new fields RequestStartTimestamp and StageTimestamp and remove ObjectMeta. This is the only option that will allow to use timestamps to make an accurate estimation of how much time the request took, since CreationTimestamp has only seconds precision.

In order to improve precision, I think no use of CreationTimestamp is a better choise. We can define two new fields RequestStartTimestamp and StageTimestamp with a more precise time metric type.

[CaoShuFeng]

/assign

[k8s-github-robot]

[MILESTONENOTIFIER] Milestone Removed

@CaoShuFeng @crassirostris

Important: This issue was missing labels required for the v1.9 milestone for more than 3 days:

kind: Must specify exactly one of kind/bug, kind/cleanup or kind/feature.

Help

• Additional instructions
• Commands for setting labels