[Enhancement] The logical operations in prehandler and posthandler are not reasonable

[lrhkobe]

Search before asking

• I had searched in the issues and found no similar issues.

Enhancement Request

Time-consuming statistics are more reasonable in posthandler.

Code in HandlerService.java:

Code in the method of preHandler:

       private void preHandler() {
            metrics.getSummaryMetrics().recordHTTPReqResTimeCost(System.currentTimeMillis() - requestTime);
            LogUtils.debug(httpLogger, "{}", response);
        }

Describe the solution you'd like

Optimize the logic in prehandler and posthandler.

Are you willing to submit PR?

• Yes I am willing to submit a PR!

Code of Conduct

• I agree to follow this project's Code of Conduct *

[github-actions]

It has been 90 days since the last activity on this issue. Apache EventMesh values the voices of the community. Please don't hesitate to share your latest insights on this matter at any time, as the community is more than willing to engage in discussions regarding the development and optimization directions of this feature.

If you feel that your issue has been resolved, please feel free to close it. Should you have any additional information to share, you are welcome to reopen this issue.

[cnzakii]

@Pil0tXia I would like to try it, please assign me.
Clarify my task: move metrics.getSummaryMetrics().recordHTTPReqResTimeCost(System.currentTimeMillis() - requestTime); in the method of preHandler to the last line in the method of postHandler.

[Pil0tXia]

@cnzakii

Including logger and any time-consuming statistics method.

It's better to finish multiple PRs one by one.

[cnzakii]

OK. I will come back to claim the issue after submitting the previous PR (if the issue still exists)

[cnzakii]

@Pil0tXia

I've already submitted the previous PR, so I think I can apply to address this issue now.

My solution:

As shown in the diagram above, move the code from the preHandler method to the postHandler method.

[Pil0tXia]

@cnzakii

As you can see that there are also some metrics related statements in postHandler(), so better put them at the top of the postHandler().

[cnzakii]

@Pil0tXia

OK. But I noticed that preHandler() is only used within run(), whereas postHandler() is used not only in run() but also in the sendResponse() method. If we move the preHandler() code into postHandler(), will it not add some unnecessary logging and time-consuming statistics?

[Pil0tXia]

The time-consuming statistics in postHandler() is acceptable, however the time recordHTTPReqResTimeCost() method recorded will be a little incorrect. Maybe creating another method and renaming these two methods if necessary will be a better way to resolve this issue.

[cnzakii]

@Pil0tXia

Could you further explain your thoughts to me? I'm not quite understanding.

Another idea I had is simply to rearrange the position of the preHandler() method.

[Pil0tXia]

@cnzakii You may wrap one method to another and call different methods under certain circumstances.

[cnzakii]

@cnzakii You may wrap one method to another and call different methods under certain circumstances.

@Pil0tXia

Is it similar to the concept of the wrapper pattern, where we create a wrapper method for postHandler(), which would perform the tasks of preHandler() within this method before calling postHandler()?

ps: Maybe the method name is not accurate

[Pil0tXia]

@cnzakii yeah 😉

[cnzakii]

@Pil0tXia

OK. Please assign the issue to me. By the way, do you have any good suggestions for naming methods?

[Pil0tXia]

@Pil0tXia You can exert your abilities and make your own decisions. After you submit a PR, I will review it again.

[cnzakii]

@Pil0tXia PTAL