Time consumed by each operator

[nishchay-veer]

This pull request enhances the codebase by introducing the model.OperatorTelemetry interface, allowing for better tracking and analysis of CPU time usage by operators.
• Added a new interface called model.OperatorTelemetry, which includes the methods AddCPUTimeTaken and CPUTimeTaken for each operator.
• The AddCPUTimeTaken method allows recording the CPU time taken by an operator during its execution.
• Included a new test TestQueryAnalyze to verify that the query analysis produces valid results, including non-zero CPU time values, for different types of queries.

[yeya24]

This only tracks the time consumed when initializing the operator. I think what we need is mainly time consumed when calling series, Next, etc

[saswatamcode]

Great start! I think we need to make a few things clear before we proceed with this.

[GiedriusS]

I would suggest just embedding model.OperatorTelemetry directly so that we wouldn't have to write these functions. It would simply be possible to call AddCPUTimeTaken directly: https://go.dev/play/p/sYvyjlMaZh0

[GiedriusS]

We should return o.next here.

[GiedriusS]

You can implement this logic once and then pass it to each operator. This way you would avoid copy/paste.

[GiedriusS]

This is the only odd Analyze(). Let's do this instead:

obsOperators := make([]model.ObservableVectorOperator, 0, len(c.operators))
for i, operator := range c.operators {
  if obsOperator, ok := operator.(model.ObservableVectorOperator); ok {
    obsOperators = append(obsOperators, obsOperator)
  }
}
return c, obsOperators

[GiedriusS]

I think in the future we will be using this struct for not just capturing CPU timing information so maybe it would be worth changing names in this PR before merging?

I suggest something like TrackedTelemetry and NoopTelemetry?

[GiedriusS]

Let's add OperatorName string like in ExplainOutputNode

[saswatamcode]

Thanks! I think now this is in a good place, some comments!

[saswatamcode]

We don't need to typecast here.

Suggested change

	name, _ := obsv.(model.VectorOperator).Explain()
	name, _ := obsv.Explain()

[GiedriusS]

Why not add the name to the original struct? This will be quite costly 😅

[saswatamcode]

This might add some additional overhead, but we can iterate on that.

[nishchay-veer]

We can return name as well in Analyze() (string, OperatorTelemetry, []ObservableVectorOperator) for each operator ?

[GiedriusS]

I think we can add the name to the struct and add a method to OperatorTelemetry like Name() string which would return the operator's name. 🤔 I think that would make the most sense

[saswatamcode]

This causes lint to fail. Let's return simply,

Suggested change

	func SetTelemetry(t model.OperatorTelemetry, opts *query.Options) {
	t = &model.NoopTelemetry{}
	if opts.EnableAnalysis {
	t = &model.TrackedTelemetry{}
	}
	func SetTelemetry(opts *query.Options) model.OperatorTelemetry {
	if opts.EnableAnalysis {
	return &model.TrackedTelemetry{}
	}
	return &model.NoopTelemetry{}

[saswatamcode]

With comment above we can then use it as,

Suggested change

	}
	SetTelemetry(r.OperatorTelemetry, opts)
	OperatorTelemetry: SetTelemetry(opts),
	}

[saswatamcode]

LGTM! Awesome work!
Let's merge and iterate on this! :)