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

Expose uri template information in a RequestInterceptor #873

Closed
pilak opened this issue Jan 10, 2019 · 13 comments
Closed

Expose uri template information in a RequestInterceptor #873

pilak opened this issue Jan 10, 2019 · 13 comments
Labels
feign-12 Issues that are related to the next major release proposal Proposed Specification or API change waiting for votes Enhancements or changes proposed that need more support before consideration

Comments

@pilak
Copy link
Contributor

pilak commented Jan 10, 2019

Hi there,

I'm looking for a way to access the request pattern url called in a logging interceptor.

Precisely, I need to get the raw url filled in @RequestLine annotation on the client class...

Is that possible ?
Best regards

@kdavisk6
Copy link
Member

Are you looking for the uri template used or the resolved value?

@kdavisk6 kdavisk6 added the question General usage or 'how-to' questions label Jan 18, 2019
@starlight36
Copy link

I have a question like this. Is there some way to access uri template from RequestTemplate? Do you have any suggestions for me?

@flisky
Copy link
Contributor

flisky commented Apr 4, 2019

@kdavisk6 The raw template uri (before resolved).

We also want to record requestTemplate.url() as a tag in a micrometer's Timer.

Currently, we have to do in the following way:

  1. wireup a interceptor to put the url into headers:
    template -> template.header(ORIGINAL_URI_HEADER, template.url())

  2. wireup a client to access the value and then remove it from headers:

String originalUrl = null;
Map<String, Collection<String>> headers = request.headers();
if (headers.containsKey(ORIGINAL_URI_HEADER)) {
    Collection<String> originalUrlHeaders = headers.get(ORIGINAL_URI_HEADER);
    if (!originalUrlHeaders.isEmpty()) {
        originalUrl = originalUrlHeaders.iterator().next();
        // recreate request because of immutable headers
        val newHeaders = new HashMap<String, Collection<String>>(headers);
        newHeaders.remove(ORIGINAL_URI_HEADER);
        request = Request.create(request.method(), request.url(), newHeaders, request.body(), request.charset());
    }
}

Any suggestions? Thanks!

@kdavisk6
Copy link
Member

@flisky

By the time the RequestTemplate makes it to the interceptor, it resolved. I'm unclear what value the unresolved uri template brings. Can you give me context?

@flisky
Copy link
Contributor

flisky commented Apr 18, 2019

Quoted from @whiskeysierra

Feign doesn't expose the URI template which significantly reduces the usefulness of the Micrometer and OpenTracing integration since both use the URI template as tags

The request would be aggregated by the unresolved uri template. And a time-series database could be exploded due to a high-cardinality tag.

@kdavisk6
Copy link
Member

@flisky

Thank you for that clarification. Is there another issue where that quote is from? I'd like to review it further to get a better understanding of what the desired outcome is.

@flisky
Copy link
Contributor

flisky commented Apr 19, 2019

It's from zalando/riptide#618.

By the way, I'm hoping there's something like context or request#attributes to hook up the uri template, rather than passing it over headers (the request headers are immutable, you know, a little bit hacky).

@flisky
Copy link
Contributor

flisky commented Apr 25, 2019

Regarding to #937, I think it's a better way to passing extra information through fragment, and fragment will be stripped off by http client automatically.

However, after a deeper investigating, the RequestTemplate is resolved before intercepting, and it means we actually cannot access the raw url template :(

Any suggestion, @kdavisk6?

@kdavisk6 kdavisk6 added proposal Proposed Specification or API change and removed question General usage or 'how-to' questions labels May 28, 2019
@kdavisk6
Copy link
Member

@flisky

Up until this point, there has been no real need to expose the uri template. I've updated this issue to reflect that we may want to consider exposing the uri template on the RequestTemplate.

For this proposal to move forward, we will need the following:

  • Votes. Simply 👍 the original comment.
  • A proposed solution. Please add a comment outlining how we could achieve this.

With those in place, the maintainers and I will work with you to narrow down the scope and agree on an approach. Once agreed, we will open this issue up for PR.

@kdavisk6 kdavisk6 added the waiting for votes Enhancements or changes proposed that need more support before consideration label May 28, 2019
@kdavisk6 kdavisk6 changed the title Is there a way to retrieve @RequestLine information in an interceptor ? Expose uri template information in a RequestInterceptor May 28, 2019
@velo
Copy link
Member

velo commented Oct 7, 2019

@pilak do you think this change would help?
https://github.com/OpenFeign/feign/pull/1091/files

@kdavisk6 kdavisk6 added the feign-12 Issues that are related to the next major release label Dec 27, 2019
@martinacat
Copy link
Contributor

Hi! I think this might be of interest: #1493

@adrian-skybaker
Copy link

Arrived at this same issue trying to do something very similar, reporting the service "resource" to Elastic APM, for which only the unresolved uri template is useful.

@pilak
Copy link
Contributor Author

pilak commented May 31, 2022

Sorry I completely missed the notification for that issue
I don't remember exactly what was the point for, but what I remember is that I needed in an interceptor to access the template (certainly in order to know which method of the client was asked for) and not the finally resolved URI.
As I understand you found a resolution for that issue and it's ok for me :)

@pilak pilak closed this as completed May 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feign-12 Issues that are related to the next major release proposal Proposed Specification or API change waiting for votes Enhancements or changes proposed that need more support before consideration
Projects
None yet
Development

No branches or pull requests

7 participants