Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support micrometer-context-propagation #5145

Open
be-hase opened this issue Aug 24, 2023 · 3 comments · May be fixed by #5577
Open

Support micrometer-context-propagation #5145

be-hase opened this issue Aug 24, 2023 · 3 comments · May be fixed by #5577
Assignees
Labels
new feature sprint Issues for OSS Sprint participants
Milestone

Comments

@be-hase
Copy link
Member

be-hase commented Aug 24, 2023

micrometer-context-propagation is now available.
https://github.com/micrometer-metrics/context-propagation

And the reactor supports micrometer-context-propagation.
https://spring.io/blog/2023/03/30/context-propagation-with-project-reactor-3-unified-bridging-between-reactive

Currently, Armeria provides RequestContextHooks, which could be replaced by providing an extension to micrometer-context-propagation.

like this:

// Sorry, I wrote it in kotlin.

class ArmeriaRequestContextAccessor : ThreadLocalAccessor<RequestContext> {
    override fun key(): Any {
        return KEY
    }

    override fun getValue(): RequestContext? {
        return RequestContextUtil.get()
    }

    override fun setValue(value: RequestContext) {
        RequestContextUtil.getAndSet<RequestContext>(value)
    }

    override fun setValue() {
        // NOOP
    }

    override fun restore(previousValue: RequestContext) {
        RequestContextUtil.getAndSet<RequestContext>(previousValue)
    }

    override fun restore() {
        RequestContextUtil.pop()
    }

    companion object {
        val KEY = ArmeriaRequestContextAccessor::class.java
    }
}

Then, users can execute the following code.

fun main(args: Array<String>) {
    ContextRegistry.getInstance().registerThreadLocalAccessor(ArmeriaRequestContextAccessor())
    Hooks.enableAutomaticContextPropagation()
    
    // run application
}

From the above article, it seems to me that the performance aspect has been devised. ( I haven't measured it but...

@jrhee17 jrhee17 added this to the 1.26.0 milestone Aug 24, 2023
@jrhee17
Copy link
Contributor

jrhee17 commented Aug 24, 2023

Took a quick look through the API/source and looks promising! 👍
If I understood correctly, this can be a much simpler solution over RequestContextHooks

@minwoox minwoox modified the milestones: 1.26.0, 1.27.0 Oct 11, 2023
@ikhoon ikhoon modified the milestones: 1.27.0, 1.28.0 Jan 16, 2024
@jrhee17 jrhee17 modified the milestones: 1.28.0, 1.29.0 Mar 19, 2024
@jrhee17 jrhee17 added the sprint Issues for OSS Sprint participants label Apr 1, 2024
@trustin
Copy link
Member

trustin commented Apr 3, 2024

@chickenchickenlove will work on it 😄

@be-hase
Copy link
Member Author

be-hase commented Apr 4, 2024

wow, sounds good.
I think it will probably improve performance.

( Sorry I didn't contribute with a pull request. haha

@chickenchickenlove chickenchickenlove linked a pull request Apr 8, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new feature sprint Issues for OSS Sprint participants
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants