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 multiple header names in middleware #85

Open
dashkan opened this issue Jun 26, 2020 · 3 comments
Open

Support multiple header names in middleware #85

dashkan opened this issue Jun 26, 2020 · 3 comments
Labels
design-required Requires API design and planning before this can be added. under consideration v4

Comments

@dashkan
Copy link

dashkan commented Jun 26, 2020

First of all, thank you for the great project. I use it in all my net core projects.

I have a unique situation where I need to integrate with legacy APIs and cannot change the source. Our new APIs use the the standard x-correlation-id header and older APIs use a different header. I need the ability to first try reading x-correlation-id and then fallback to old header name.

Are you open to extending the middleware via the CorrelationIdOptions by adding a callback property

        /// <summary>
        /// A callback to resolve the correlation ID. 
        /// </summary>
        public Func<IHeaderDictionary, StringValues> RequestHeaderCallback { get; set; }

And update the middleware

            StringValues cid;
            if (_options.RequestHeaderCallback != null)
                cid = _options.RequestHeaderCallback(context.Request.Headers);
            else
                context.Request.Headers.TryGetValue(_options.RequestHeader, out cid);
            
            var hasCorrelationIdHeader = !StringValues.IsNullOrEmpty(cid);

This simple change would allow users to use custom logic to read the correlation header. I can even make the callback more broad by passing the HttpContext and maybe even IServiceProvider in case people need to use DI to get their correlation id value.

Happy to submit a PR.

@stevejgordon
Copy link
Owner

Hi @dashkan. Thanks for raising this suggestion and the offer of a PR!

This feels like a useful addition for situations such as yours. I'm wondering if it would be enough to simply support an array of header names to attempt to read, with the first match returning the correlation ID?

@stevejgordon stevejgordon added design-required Requires API design and planning before this can be added. under consideration labels Jul 14, 2020
@dashkan
Copy link
Author

dashkan commented Jul 18, 2020

Hi @stevejgordon. That would work definitely work for me. I was just trying to expand the api to cover more complex workflows.

Totally understand if you want to keep it simple.

Happy to submit PR in either case.

@wdolek
Copy link

wdolek commented Sep 1, 2020

If I may suggest, this could be also done by providing sequence of headers to be used when reading correlation ID, and then simply iterating over it and trying to find given header in request. Assuming that such iteration won't be expensive since you will configure just few headers, and lookup in HttpRequest.Headers is cheap as it is dictionary.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
design-required Requires API design and planning before this can be added. under consideration v4
Projects
None yet
Development

No branches or pull requests

3 participants