feat: Micrometer Observation instrumentation

[marcingrzejszczak]

Description

• adds a VaadinFilter mechanism to mimic an around-aspect
• adds a Micrometer Observation implementation
• configures VaadinServlet to automatically update the VaadinService with filters
• configures Spring to automatically update the VaadinServlet with filters

What's missing:

• Client side instrumentation
• Tests
• Documentation

Fixes #17436

Type of change

• Bugfix
• Feature

Checklist

• I have read the contribution guide: https://vaadin.com/docs/latest/guide/contributing/overview/
• I have added a description following the guideline.
• The issue is created in the corresponding repository and I have referenced it.
• I have added tests to ensure my change is effective and works as intended.
• New and existing tests are passing locally with my change.
• I have performed self-review and corrected misspellings.

Additional for Feature type of change

• Enhancement / new feature was discussed in a corresponding GitHub issue and Acceptance Criteria were created.

[CLAassistant]

All committers have signed the CLA.

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[mshabarov]

@marcingrzejszczak thank you a lot for your contribution, we appreciate it and will start reviewing it soon 👍

[knoobie]

I'm wondering if it would be worth to focus first on this interface and its integration into flow, allowing this to be already added e.g. in the upcoming release - especially since the full integration could take a little bit longer (see Jakarta / Snapshot usage). If I remember correctly, I've seen multiple dicussions quite recently where people needed hooks like this. Shamelessly pinging @mstahv

[marcingrzejszczak]

Sure! I didn't know if you want sth like that. Let me split the two then.

[knoobie]

wait for Mikhail's response before you do anything. I'm just an external contributor like you ;)

[marcingrzejszczak]

I split that #17502, I still think the change makes sense to be split into 2 PRs. Thanks for the great tip

[mstahv]

@knoobie Already subscribed 😎

Quick comment on the VaadinFilter (although now in separate PR): maybe it could also cover UI.access calls coming from separate threads 🤔 And then requests & al are optional or within some context parameter 🤷‍♂️

[mshabarov]

I'd propose to extract the Micrometer Observation implementation of request interceptors into a separate maven module flow-observation or flow-micrometer.
Then the Micrometer dependencies can be normal, i.e. non-optional.
Then we need to add it into flow-bom and into vaadin-bom on the platform side.
Finally, it can be pluggable, so if a project defines

            <dependency>
                <groupId>com.vaadin</groupId>
                <artifactId>flow-micrometer</artifactId>
            </dependency>

then the Micrometer's VaadinRequestInterceptors add themselves via Service Init Listener. Otherwise, if no dependency is set, Micrometer observation isn't included.

We did something similar with flow-webpush module recently, so you can use it as an example.

[mshabarov]

Needs a rebase to main branch.

[marcingrzejszczak]

@mshabarov I wonder if we shouldn't first add instrumentation of the commands. To this, we need to implement this micrometer-metrics/micrometer#3989 and that's not merged yet

[mshabarov]

I wonder if we shouldn't first add instrumentation of the commands. To this, we need to implement this micrometer-metrics/micrometer#3989 and that's not merged yet

I would suggest to add first the instrumentation for things that already merge and for what we already have a PR. So could we merge the instrumentation proposed in this PR?

[marcingrzejszczak]

I would love to but this https://github.com/vaadin/flow/pull/17494/files#diff-4abe9f264d9d9616e9178b98b70fdfbb13aeea2d49ef4c0e6c30f04882ab55d3R147 is not actually pushed anywhere. I made it work locally because I installed this module in my local .m2. Before this PR gets merged, this PR micrometer-metrics/micrometer#3989 must get merged first 😬

[knoobie]

Just wondering is this targeted for Micrometer 1.12 / Spring Boot 3.2 in November or is it pending a release version, meaning it could at first land in 1.1x / Spring Boot 3.3 in Mai 2024? ;)

[marcingrzejszczak]

I want the PR to merged asap so that it's happening for 1.12 of Micrometer.

[mshabarov]

I see, thanks for explanations 👍
So we have to wait a bit, no problem.

[marcingrzejszczak]

Do we want to proceed with the command instrumentation in the meantime? Also do you have a chat of some sort (e.g. Slack) where it will be easier to communicate rather than Github issues? :)

[mshabarov]

@marcingrzejszczak

Do we want to proceed with the command instrumentation in the meantime?

Yes, let's do. I remember you had a branch with prototype. Could you share it with a new pull request and I'll review?

Also do you have a chat of some sort (e.g. Slack) where it will be easier to communicate rather than Github issues? :)

Discord is it :) Here is a link https://discord.com/invite/vaadin. You can DM me directly there.

[marcingrzejszczak]

@mshabarov there you go #17738