Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Support Prometheus_client 0.4.0+ #5636

Merged
merged 13 commits into from
Jul 18, 2019
Merged

Support Prometheus_client 0.4.0+ #5636

merged 13 commits into from
Jul 18, 2019

Conversation

hawkowl
Copy link
Contributor

@hawkowl hawkowl commented Jul 8, 2019

This also makes it the minimum, since it's easier than two different implementations.

It supports both the new format and the old format of naming.

Fixes #4221

@hawkowl hawkowl requested a review from a team July 8, 2019 15:26
Copy link
Member

@richvdh richvdh left a comment

Choose a reason for hiding this comment

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

looks generally good to me.

What do you think our long-term plan should be here? can we deprecate the old metrics names and go back to using the stock prometheus code?

synapse/metrics/_exposition.py Show resolved Hide resolved
synapse/metrics/_exposition.py Show resolved Hide resolved
@hawkowl
Copy link
Contributor Author

hawkowl commented Jul 10, 2019

@richvdh the goal is probably to migrate to the new ones and then ditch this and move upstream. I'm not sure if that's a day that'll ever come, but...

@richvdh richvdh self-requested a review July 11, 2019 08:55
@richvdh
Copy link
Member

richvdh commented Jul 11, 2019

the goal is probably to migrate to the new ones and then ditch this and move upstream. I'm not sure if that's a day that'll ever come, but...

yup fair. it's good to have a plan, anyway.

As you mentioned in the meeting, some notes about migrating to new metrics would be useful - see https://github.com/matrix-org/synapse/blob/master/docs/metrics-howto.rst for how this sort of thing has been handled in the past.

The code LGTM other than #5636 (comment).

@codecov
Copy link

codecov bot commented Jul 11, 2019

Codecov Report

Merging #5636 into develop will decrease coverage by 0.01%.
The diff coverage is 63.15%.

@@             Coverage Diff             @@
##           develop    #5636      +/-   ##
===========================================
- Coverage    63.25%   63.24%   -0.02%     
===========================================
  Files          328      328              
  Lines        35854    35957     +103     
  Branches      5911     5935      +24     
===========================================
+ Hits         22681    22742      +61     
- Misses       11548    11582      +34     
- Partials      1625     1633       +8

@codecov
Copy link

codecov bot commented Jul 11, 2019

Codecov Report

Merging #5636 into develop will increase coverage by <.01%.
The diff coverage is 62.87%.

@@             Coverage Diff             @@
##           develop    #5636      +/-   ##
===========================================
+ Coverage    63.42%   63.43%   +<.01%     
===========================================
  Files          331      331              
  Lines        36163    36284     +121     
  Branches      5960     5988      +28     
===========================================
+ Hits         22938    23015      +77     
- Misses       11597    11631      +34     
- Partials      1628     1638      +10

@richvdh richvdh self-requested a review July 12, 2019 11:26
@@ -74,7 +89,18 @@ def sample_line(line, name):
)


def generate_latest(registry):
def nameify_sample(sample):
Copy link
Member

Choose a reason for hiding this comment

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

for the record, if we have to actively do stuff to support old versions of deps, I'm quite happy for us to drop compatibility with those old versions, so in this case dropping support for promethus_cleint<0.4 would have been fine.

I'm just asking that we don't bump our min dependencies as a matter-of-course.

Anyway, this looks good.

Copy link
Member

@richvdh richvdh left a comment

Choose a reason for hiding this comment

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

metrics-howto.rst looks great, but we should probably link to it from UPGRADE.rst and the changelog.

changelog.d/5636.misc Outdated Show resolved Hide resolved
@hawkowl hawkowl merged commit 7ad1d76 into develop Jul 18, 2019
@hawkowl hawkowl deleted the hawkowl/new_promclient branch July 18, 2019 13:57
anoadragon453 added a commit that referenced this pull request Jul 22, 2019
v1.2.0rc1

Features
--------

- Add support for opentracing. ([\#5544](#5544), [\#5712](#5712))
- Add ability to pull all locally stored events out of synapse that a particular user can see. ([\#5589](#5589))
- Add a basic admin command app to allow server operators to run Synapse admin commands separately from the main production instance. ([\#5597](#5597))
- Add `sender` and `origin_server_ts` fields to `m.replace`. ([\#5613](#5613))
- Add default push rule to ignore reactions. ([\#5623](#5623))
- Include the original event when asking for its relations. ([\#5626](#5626))
- Implement `session_lifetime` configuration option, after which access tokens will expire. ([\#5660](#5660))
- Return "This account has been deactivated" when a deactivated user tries to login. ([\#5674](#5674))
- Enable aggregations support by default ([\#5714](#5714))

Bugfixes
--------

- Fix 'utime went backwards' errors on daemonization. ([\#5609](#5609))
- Various minor fixes to the federation request rate limiter. ([\#5621](#5621))
- Forbid viewing relations on an event once it has been redacted. ([\#5629](#5629))
- Fix requests to the `/store_invite` endpoint of identity servers being sent in the wrong format. ([\#5638](#5638))
- Fix newly-registered users not being able to lookup their own profile without joining a room. ([\#5644](#5644))
- Fix bug in #5626 that prevented the original_event field from actually having the contents of the original event in a call to `/relations`. ([\#5654](#5654))
- Fix 3PID bind requests being sent to identity servers as `application/x-form-www-urlencoded` data, which is deprecated. ([\#5658](#5658))
- Fix some problems with authenticating redactions in recent room versions. ([\#5699](#5699), [\#5700](#5700), [\#5707](#5707))
- Ignore redactions of m.room.create events. ([\#5701](#5701))

Updates to the Docker image
---------------------------

- Base Docker image on a newer Alpine Linux version (3.8 -> 3.10). ([\#5619](#5619))
- Add missing space in default logging file format generated by the Docker image. ([\#5620](#5620))

Improved Documentation
----------------------

- Add information about nginx normalisation to reverse_proxy.rst. Contributed by @skalarproduktraum - thanks! ([\#5397](#5397))
- --no-pep517 should be --no-use-pep517 in the documentation to setup the development environment. ([\#5651](#5651))
- Improvements to Postgres setup instructions. Contributed by @Lrizika - thanks! ([\#5661](#5661))
- Minor tweaks to postgres documentation. ([\#5675](#5675))

Deprecations and Removals
-------------------------

- Remove support for the `invite_3pid_guest` configuration setting. ([\#5625](#5625))

Internal Changes
----------------

- Move logging code out of `synapse.util` and into `synapse.logging`. ([\#5606](#5606), [\#5617](#5617))
- Add a blacklist file to the repo to blacklist certain sytests from failing CI. ([\#5611](#5611))
- Make runtime errors surrounding password reset emails much clearer. ([\#5616](#5616))
- Remove dead code for persiting outgoing federation transactions. ([\#5622](#5622))
- Add `lint.sh` to the scripts-dev folder which will run all linting steps required by CI. ([\#5627](#5627))
- Move RegistrationHandler.get_or_create_user to test code. ([\#5628](#5628))
- Add some more common python virtual-environment paths to the black exclusion list. ([\#5630](#5630))
- Some counter metrics exposed over Prometheus have been renamed, with the old names preserved for backwards compatibility and deprecated. See `docs/metrics-howto.rst` for details. ([\#5636](#5636))
- Unblacklist some user_directory sytests. ([\#5637](#5637))
- Factor out some redundant code in the login implementation. ([\#5639](#5639))
- Update ModuleApi to avoid register(generate_token=True). ([\#5640](#5640))
- Remove access-token support from `RegistrationHandler.register`, and rename it. ([\#5641](#5641))
- Remove access-token support from `RegistrationStore.register`, and rename it. ([\#5642](#5642))
- Improve logging for auto-join when a new user is created. ([\#5643](#5643))
- Remove unused and unnecessary check for FederationDeniedError in _exception_to_failure. ([\#5645](#5645))
- Fix a small typo in a code comment. ([\#5655](#5655))
- Clean up exception handling around client access tokens. ([\#5656](#5656))
- Add a mechanism for per-test homeserver configuration in the unit tests. ([\#5657](#5657))
- Inline issue_access_token. ([\#5659](#5659))
- Update the sytest BuildKite configuration to checkout Synapse in `/src`. ([\#5664](#5664))
- Add a `docker` type to the towncrier configuration. ([\#5673](#5673))
- Convert `synapse.federation.transport.server` to `async`. Might improve some stack traces. ([\#5689](#5689))
- Documentation for opentracing. ([\#5703](#5703))
anoadragon453 added a commit that referenced this pull request Feb 19, 2020
okurz added a commit to okurz/synapse that referenced this pull request Jul 3, 2020
Fixes matrix-org#7641

The package was pinned to <0.8.0 without an obvious reasoning with
7ad1d76
in matrix-org#5636
while the version selection looks to just try to exclude an arbitrary
next minor version number that might introduce API breaking changes.
Selecting the next minor number might be a good conservative selection.

Downstream distributions already reported success patching out the version
requirements.

This also fixes the integration of upgraded packages into openSUSE packages,
e.g. for openSUSE Tumbleweed which already ships prometheus_client >= 0.8 .

Signed-off-by: Oliver Kurz <okurz@suse.de>
richvdh added a commit that referenced this pull request Jul 6, 2020
Fixes #7641

The package was pinned to <0.8.0 without an obvious reasoning with
7ad1d76
in #5636
while the version selection looks to just try to exclude an arbitrary
next minor version number that might introduce API breaking changes.
Selecting the next minor number might be a good conservative selection.

Downstream distributions already reported success patching out the version
requirements.

This also fixes the integration of upgraded packages into openSUSE packages,
e.g. for openSUSE Tumbleweed which already ships prometheus_client >= 0.8 .

Signed-off-by: Oliver Kurz <okurz@suse.de>

Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants