Refactor the extended MetricExtension interface to match fundamental events in Sentinel

[sczyh30]

Describe what this PR does / why we need it

Refactor the extended AdvancedMetricExtension interface (new added in #1665) to match fundamental events (pass, block, complete) in Sentinel.

Does this pull request fix one issue?

Improvement for #1665

Describe how you did it

• Unify the extended interface as a few event handlers: onPass, onBlocked, onComplete and onError
• Polish related code

Describe how to verify it

Run all test cases for regression.

Special notes for reviews

NONE

[jasonjoo2010]

I have done a quick overview on this pr.
First as a background the previous contribution has never ever been released (#1665), right?

So my concern is that why we insist on keeping the inheritance between event style interface and counter style interface(legacy one)? Since anyone who wants to implement "Advanced" one must implement interfaces, addPass/onPass, addBlock/onBlock, etc., altogether even he only want to follow the method group of event style. Is there any constraint we must merge them into a base, non-empty interface type?

Normally and mostly we may use an empty base interface for referencing and do type detection later. Sure for backward compatibility this way should not be used.

Another thought is that we can separate them into independent interfaces and maintain in two registries or collections.

I will continue to do a quick review after we make these clear.

[sczyh30]

I have done a quick overview on this pr.
First as a background the previous contribution has never ever been released (#1665), right?

Yes, it's not released yet.

So my concern is that why we insist on keeping the inheritance between event style interface and counter style interface(legacy one)? Since anyone who wants to implement "Advanced" one must implement interfaces, addPass/onPass, addBlock/onBlock, etc., altogether even he only want to follow the method group of event style. Is there any constraint we must merge them into a base, non-empty interface type?

Normally and mostly we may use an empty base interface for referencing and do type detection later. Sure for backward compatibility this way should not be used.

Another thought is that we can separate them into independent interfaces and maintain in two registries or collections.

This is only a temporary workaround for backward compatibility (as some other libraries has been integrating with the SPI). Actually I was planning to deprecate the legacy MetricExtension hierarchy in upcoming versions and make the metric hook as the "canonical" event style.

[jasonjoo2010]

This is only a temporary workaround for backward compatibility (as some other libraries has been integrating with the SPI). Actually I was planning to deprecate the legacy MetricExtension hierarchy in upcoming versions and make the metric hook as the "canonical" event style.

Understood.
So how does the canonical look like? Is it still in designing?
If so we may mainly do some renaming in this pr and abandon it after a few iterations, right?

[sczyh30]

So how does the canonical look like? Is it still in designing?

It's not settled down. Discussions are needed later.

If so we may mainly do some renaming in this pr and abandon it after a few iterations, right?

That's it.

[jasonjoo2010]

So how does the canonical look like? Is it still in designing?

It's not settled down. Discussions are needed later.

If so we may mainly do some renaming in this pr and abandon it after a few iterations, right?

That's it.

Got it now and I have no further question.

[jasonjoo2010]

LGTM