Skip to content

Conversation

MadLittleMods
Copy link
Contributor

@MadLittleMods MadLittleMods commented Jul 24, 2025

Update metrics linting to be able to handle custom metrics

Part of #18592

Dev notes

  • GaugeBucketCollector
  • LaterGauge

mypy extensions

Mypy Bug /limitation where multiple extensions using get_base_class_hook interfere with each other python/mypy#19524

Todo

Pull Request Checklist

  • Pull request is based on the develop branch
  • Pull request includes a changelog file. The entry should:
    • Be a short description of your change which makes sense to users. "Fixed a bug that prevented receiving messages from other servers." instead of "Moved X method from EventStore to EventWorkerStore.".
    • Use markdown where necessary, mostly for code blocks.
    • End with either a period (.) or an exclamation mark (!).
    • Start with a capital letter.
    • Feel free to credit yourself, by adding a sentence "Contributed by @github_username." or "Contributed by [Your Name]." to the end of the entry.
  • Code style is correct (run the linters)

```
synapse/metrics/__init__.py:522: error: Expected the `labelnames` argument of Histogram to be a list of label names (including `SERVER_NAME_LABEL`), but got TupleExpr:528(
  StrExpr(type)
  StrExpr(reason)
  NameExpr(SERVER_NAME_LABEL [synapse.metrics.SERVER_NAME_LABEL])). If this is a process-level metric (vs homeserver-level), use a type ignore comment to disable this check.  [missing-server-name-label]
```
Conflicts:
	scripts-dev/mypy_synapse_plugin.py
@MadLittleMods MadLittleMods changed the title Updating metrics linting to be able to handle custom metrics Update metrics linting to be able to handle custom metrics Jul 28, 2025
@MadLittleMods MadLittleMods marked this pull request as ready for review July 29, 2025 00:47
@MadLittleMods MadLittleMods requested a review from a team as a code owner July 29, 2025 00:47
Conflicts:
	scripts-dev/mypy_synapse_plugin.py
@devonh
Copy link
Member

devonh commented Jul 30, 2025

I believe this PR looks good to merge as is.
But I think we should discuss with the team first to make sure everyone is in agreement that this makes sense to add.
As long as others are aware that we are intentionally creating a lint that will disappear into oblivion and rarely, if ever, get run, then I am happy to bring it into the codebase.

@MadLittleMods MadLittleMods requested review from devonh and removed request for a team July 31, 2025 23:20
@MadLittleMods MadLittleMods requested a review from devonh August 1, 2025 17:48
Copy link
Member

@devonh devonh left a comment

Choose a reason for hiding this comment

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

Thanks for pushing through to get this running by default!

@MadLittleMods MadLittleMods merged commit e16fbdc into develop Aug 1, 2025
44 checks passed
@MadLittleMods MadLittleMods deleted the madlittlemods/18592-lint-metric branch August 1, 2025 20:34
@MadLittleMods
Copy link
Contributor Author

MadLittleMods commented Aug 1, 2025

Thanks for the review @devonh and @frebib for the suggestion to make the lint viable 🦛

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants