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

How to get client's real ip? #507

Open
bangbaew opened this issue Mar 13, 2023 · 18 comments
Open

How to get client's real ip? #507

bangbaew opened this issue Mar 13, 2023 · 18 comments
Labels
👍 Accepting PR 💡 Help wanted Extra attention is needed 🤔 Question Further information is requested

Comments

@bangbaew
Copy link

image

When make a remote request to my Gofiber endpoint, it gives http.client_ip = 10.8.11.189, which is container's local ip, but in Rust version of opentelemetry, used with Actix Web, it gives my real public ip out there, how can I make Gofiber's otel show public client ip?

@gaby
Copy link
Member

gaby commented Mar 13, 2023

@bangbaew Is the rust version also running inside Docker?

@ReneWerner87 ReneWerner87 added the 🤔 Question Further information is requested label Mar 13, 2023
@gaby
Copy link
Member

gaby commented Mar 13, 2023

Found the issue. We are using the ClientIP from the context here: https://github.com/gofiber/contrib/blob/main/otelfiber/semconv.go#L59

We need to add support for X-Forwarded-For.

Related issue: open-telemetry/opentelemetry-go#2282

I do think this should probably be fixed in Fiber instead of the middleware. Someone reported a similar issue when using c.IP() a few days ago on discord.

@bangbaew
Copy link
Author

@bangbaew Is the rust version also running inside Docker?

It's running inside a container, same network as the Gofiber app.
This is the Rust library i use: https://github.com/OutThereLabs/actix-web-opentelemetry

@gaby
Copy link
Member

gaby commented Mar 13, 2023

@bangbaew Is the rust version also running inside Docker?

It's running inside a container, same network as the Gofiber app. This is the Rust library i use: https://github.com/OutThereLabs/actix-web-opentelemetry

Yeah, this is a Fiber bug.

@gaby
Copy link
Member

gaby commented Mar 13, 2023

We can probably solve this by using this: https://docs.gofiber.io/api/ctx#ips

@bangbaew
Copy link
Author

@bangbaew Is the rust version also running inside Docker?

It's running inside a container, same network as the Gofiber app. This is the Rust library i use: https://github.com/OutThereLabs/actix-web-opentelemetry

Yeah, this is a Fiber bug.

Yeah, the log IPs on the terminal as well, they all are local IPs, and I don't think they're any useful.
image

@gaby
Copy link
Member

gaby commented Mar 13, 2023

@bangbaew Is the rust version also running inside Docker?

It's running inside a container, same network as the Gofiber app. This is the Rust library i use: https://github.com/OutThereLabs/actix-web-opentelemetry

Yeah, this is a Fiber bug.

Yeah, the log IPs on the terminal as well, they all are local IPs, and I don't think they're any useful. image

Those are expected since thats your IP inside the container. They only way to get the real IP in the logs is by parsing the Forwarded headers, it should be the first one in the List.

In one of your routes log ctx.IPs()

@ReneWerner87
Copy link
Member

clientIP := c.IP()
if len(clientIP) > 0 {
attrs = append(attrs, semconv.HTTPClientIPKey.String(utils.CopyString(clientIP)))
}

https://github.com/gofiber/fiber/blob/634f163e3f6292e658e61d0dd9e3c475d87b5d54/ctx.go#L699-L701

https://docs.gofiber.io/next/api/fiber#config
image

did you configure this header ? otherwise the fiber app can not determine the real ip

@gaby maybe we should extend the doc for these cases (ip method)

@ReneWerner87
Copy link
Member

@gaby
Copy link
Member

gaby commented Mar 13, 2023

clientIP := c.IP()
if len(clientIP) > 0 {
attrs = append(attrs, semconv.HTTPClientIPKey.String(utils.CopyString(clientIP)))
}

https://github.com/gofiber/fiber/blob/634f163e3f6292e658e61d0dd9e3c475d87b5d54/ctx.go#L699-L701

https://docs.gofiber.io/next/api/fiber#config image

did you configure this header ? otherwise the fiber app can not determine the real ip

@gaby maybe we should extend the doc for these cases (ip method)

Agree, it's a bit confusing. From a otelfiber perspective using c.IPs() may be better since opentelemetry will auto-parse the list and only use the first IP which is the real client IP.

@ReneWerner87
Copy link
Member

@bangbaew have you ever tested what you get when you configure the header of the proxy (mostly forwarded-for ) in your fiber app ?

@bangbaew
Copy link
Author

bangbaew commented Mar 14, 2023

@bangbaew have you ever tested what you get when you configure the header of the proxy (mostly forwarded-for ) in your fiber app ?

If you mean have I tried logging from C.IPs() and c.GetReqHeaders(), I've tried them and the real IPs are shown in the fmt.Println, they both echo the X-Forwarded-For
If I send a request over Kong Gateway endpoint, it will log this

"X-Forwarded-For": "{my real public ip}, 10.8.26.4",
"X-Real-Ip": "10.8.26.4"

The 10.8.26.4 is Kong instance's IP.

If I send a request directly, it will log this

"X-Forwarded-For": "{my real public ip}",
"X-Real-Ip": "{my real public ip}"

but both of them will log this in Jaeger UI

http.client_ip 10.8.51.49

You can see that the http.client_ip in Jaeger UI is the fiber instance's local ip, not even the forwarded IPs.

But I don't know how to configure the header of the proxy in my fiber app.

@ReneWerner87
Copy link
Member

ReneWerner87 commented Mar 14, 2023

But I don't know how to configure the header of the proxy in my fiber app.

@bangbaew like this

app := fiber.New(fiber.Config{
	ProxyHeader: fiber.HeaderXForwardedFor,
})

https://docs.gofiber.io/next/api/fiber#config
image

@bangbaew
Copy link
Author

But I don't know how to configure the header of the proxy in my fiber app.

@bangbaew like this

app := fiber.New(fiber.Config{
	ProxyHeader: fiber.HeaderXForwardedFor,
})

https://docs.gofiber.io/next/api/fiber#config image

Thanks a lot! it shows the X-Forwarded-For IPs now, with both public IP and API Gateway's IP, can I make it record only the first value?

@ReneWerner87
Copy link
Member

ReneWerner87 commented Mar 14, 2023

do not think so, I would have to research

in any case, we should expand the documentation

@bangbaew you can do that, you know best where you searched for the solution of the problem

maybe in the examples and as a hint in the readme
https://github.com/gofiber/contrib/tree/main/otelfiber#readme

@ReneWerner87
Copy link
Member

ReneWerner87 commented Mar 14, 2023

@gaby
Copy link
Member

gaby commented Mar 15, 2023

@bangbaew opentelemetry says they only take the first value. Has that been the case for you after adding the header?

@ReneWerner87
Copy link
Member

maybe we can change the middleware and cut away the second value which comes back through the header

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
👍 Accepting PR 💡 Help wanted Extra attention is needed 🤔 Question Further information is requested
Projects
None yet
Development

No branches or pull requests

3 participants