-
Notifications
You must be signed in to change notification settings - Fork 30
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
Path for Rocket Analytics should be the route name. #20
Comments
Or, maybe you could allow the user to pass in a closure that computes some of the fields. I run my services in Again, I am willing to contribute on that, as well, but want to get your thoughts first. |
Hi! Thanks a lot, great suggestions! I could think of some situations in which capturing the true path would be beneficial, e.g. identifying the most active user, so maybe this could be an optional flag that you pass into the middleware at the same time you provide the API key. Maybe a The closures would be a very cool upgrade but now you've mentioned it I think in general the IP address should be changed so it attempts to take from headers first and if not resorts to the IP from the gateway device. I think we're going to need an optional I'm very happy for you to make these upgrades and I'll check them out and merge. If they work well I'll try and mirror them on actix and axum |
Cool, sounds good. One issue with the IP is there are some serverless platforms like |
Ah, I didn't realise that! Passing in a closure is definitely for the todo list then. Is it only the IP address that would be affected by this? |
Generally, yes. Let me take a stab at it today, and you can let me know if
you think it's reasonable. :)
Aaron
…On Thu, Aug 17, 2023 at 12:47 AM Tom Draper ***@***.***> wrote:
Ah, I didn't realise that! Passing in a closure is definitely for the todo
list then. Is it only the IP address that would be affected by this?
—
Reply to this email directly, view it on GitHub
<#20 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAEGFE6WEJET76HMXAWYVYDXVXEBJANCNFSM6AAAAAA3RPQIWI>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
Ok, here is a pretty good way to implement what I was thinking. I am deploying using my client on a running service. Feel free to hit me up on Discord ('twitchax`) if you want to discuss. |
Ohhh, I just realized having a I want to filter away my health checks, so we could add a |
Ok, I see why the response times are so low. Most of my handlers return within the low microsecond, sometimes nanosecond, timeframe, so the |
Also, some I'd be curious to make this |
Compression is definitely a great upgrade but I haven't had time to get around to it yet. At the moment I'm using a basic postgresql database but I've always thought migration to a time series database like timescaledb would be a big improvement. I've never used SurrealDB, but it looks like it could be cool. Do you mean response times when loading the dashboard? I'm fairly ~90% is due to rendering the graphs, especially with a large amount of requests logged. |
Yeah, the network call to the backend takes about 4 seconds. The frontend render is nearly instantaneous. |
Hey, thanks a lot for your help and sorry about the delay I've had a lot going on. I've merged your pull request, but I've removed the user configurable batch_length_seconds for the time being to avoid the potential for abuse/overloading the server or causing API performance issues due to frequent thread creation. The only downside to this is that the dashboard data can be up to 60 seconds delayed which I think is still acceptable. This would be a good one to re-include in the future when the server is more stable and I have a more scalable storage solution. |
Hello!
Awesome project. For the rocket analytics, the "true path" is recorded. This ends up yielding endpoints in the UI like
/api/things/some_unique_id
, rather than likely the grouping that most people would want, like/api/things/<id>
, which is the route name.This would make all calls to
/api/things/<id>
grouped in the UI, which, I believe, is how most people would like to see the data grouped.This can be fixed for rocket by encoding the route rather than the path (e.g.,
req.route().as_ref().uri.unwrap_or_else("UNKNOWN")
).I'm happy to contribute, but I wanted to verify that you are OK with this.
The text was updated successfully, but these errors were encountered: