Skip to content

Added optional middleware param to emulator start for metrics#815

Merged
mfbz merged 2 commits intomasterfrom
mfbz/add-middleware-param
Apr 16, 2025
Merged

Added optional middleware param to emulator start for metrics#815
mfbz merged 2 commits intomasterfrom
mfbz/add-middleware-param

Conversation

@mfbz
Copy link
Copy Markdown
Contributor

@mfbz mfbz commented Apr 15, 2025

Closes #812.

Description

Added an optional http middleware parameter that can be added to rest api server. The goal is being able to use analytics to get some developers metrics as per onflow/flow-cli#1919.

Comment thread cmd/emulator/start/start.go Outdated
func Cmd(getServiceKey serviceKeyFunc) *cobra.Command {
type HttpMiddleware func(http.Handler) http.Handler

func Cmd(getServiceKey serviceKeyFunc, httpMiddlewares ...HttpMiddleware) *cobra.Command {
Copy link
Copy Markdown
Contributor

@jribbink jribbink Apr 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Personally I would prefer that we switch this to a config struct, i.e.

type Config struct {
    GetServiceKey serviceKeyFunc
    HTTPMiddlewares []HttpMiddleware 
}

It is a breaking change, but it shouldn't be a big deal an it keeps things more extensible for the future.

Comment thread cmd/emulator/start/start.go Outdated
Comment on lines +223 to +225
if len(httpMiddlewares) > 0 {
emu.UseRestMiddleware(httpMiddlewares[0])
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if len(httpMiddlewares) > 0 {
emu.UseRestMiddleware(httpMiddlewares[0])
}
for _, middleware := range httpMiddlewares {
emu.UseRestMiddleware(middleware)
}

@mfbz mfbz requested a review from jribbink April 15, 2025 16:26
@mfbz mfbz merged commit 61098d6 into master Apr 16, 2025
3 checks passed
@mfbz mfbz deleted the mfbz/add-middleware-param branch April 16, 2025 15:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add middleware for analytics

2 participants