Add Prometheus middleware for gundeck, cannon, cargohold, brig, proxy #672
Conversation
ChrisPenner
changed the title
ChrisPenner
force-pushed
the
cp/prometheus-middleware
branch
from
cc8bc24 to
94fe8e7
Compare
Note that we don't have any integration tests for cannon; Adding the whole integration flow for cannon is a bunch of overhead for a pretty simple and mostly unimportant test so Me and Joé decided to skip it.
ChrisPenner
force-pushed
the
cp/prometheus-middleware
branch
from
94fe8e7 to
fcbb64e
Compare
|
i feel that explicit exports are too restrictive. it should be clear from the module name what's on the interface and what's internal (eg., by naming modules 'Internal'). i've been kept from doing reasonable things to libraries by explicit export lists too much in the past. but i may be wrong with all of this.
…On Mon, Mar 25, 2019 at 07:06:07AM -0700, Chris Penner wrote:
Date: Mon, 25 Mar 2019 07:06:07 -0700
From: Chris Penner ***@***.***>
To: wireapp/wire-server ***@***.***>
Cc: fisx ***@***.***>, Comment ***@***.***>
Subject: Re: [wireapp/wire-server] Add Prometheus middleware for gundeck,
cannon, cargohold, brig, proxy (#672)
ChrisPenner commented on this pull request.
> @@ -47,6 +47,7 @@ library:
- Brig.Options
- Brig.Provider.DB
- Brig.RPC
+ - Brig.Run
I was wondering about that; wasn't sure whether it was a pattern or just something left over; in certain cases it might be nice to know what's explicitly accessible; but maybe not? What do you think?
--
You are receiving this because you commented.
Reply to this email directly or view it on GitHub:
#672 (comment)
--
|
|
👍 |
|
@fisx (re: explicit imports) I can definitely see that being a problem in certain cases; I like them because they shield the importer from all of your helper functions polluting the namespace; as well as being a cheap form of module documentation (you can quickly see what the purpose of the module is and what it contains. I'm open to following a standard if we have one; but it sounds like it's kinda wishy washy right now. |
I think I was talking about exported modules in package.yaml (even though my wording was poor). I agree in principle that explicit names exports in modules are good, but I prefer having an |
There are some test refactorings in here; feel free to read commit-by-commit.
I took the opportunity to try to standardize a few things across all services. Now all services have a consistent
Runmodule with a separated middleware stack.When adding tests I tried to stay mostly consistent with the project's style; so some of the tests may look different from others.