Skip to content

Add MetricsReporter interface so that databaseBackend's can share their connection counts with CensusManager - #31269

Merged
elliesterner merged 5 commits into
mainfrom
VAULT-32655/add-reporter-interface
Jul 14, 2025
Merged

Add MetricsReporter interface so that databaseBackend's can share their connection counts with CensusManager#31269
elliesterner merged 5 commits into
mainfrom
VAULT-32655/add-reporter-interface

Conversation

@elliesterner

@elliesterner elliesterner commented Jul 14, 2025

Copy link
Copy Markdown
Contributor

Description

This PR adds a MetricsReporter interface so that core can fetch the database backend's connection counts when reporting product usage metrics to census manager.

I'm going to add the changelog to the follow up PR that actually uses this new interface. This PR just defines it and includes a test.

This will go out in 1.21.0 and won't be backported.

TODO only if you're a HashiCorp employee

  • Backport Labels: If this fix needs to be backported, use the appropriate backport/ label that matches the desired release branch. Note that in the CE repo, the latest release branch will look like backport/x.x.x, but older release branches will be backport/ent/x.x.x+ent.
    • LTS: If this fixes a critical security vulnerability or severity 1 bug, it will also need to be backported to the current LTS versions of Vault. To ensure this, use all available enterprise labels.
  • ENT Breakage: If this PR either 1) removes a public function OR 2) changes the signature
    of a public function, even if that change is in a CE file, double check that
    applying the patch for this PR to the ENT repo and running tests doesn't
    break any tests. Sometimes ENT only tests rely on public functions in CE
    files.
  • Jira: If this change has an associated Jira, it's referenced either
    in the PR description, commit message, or branch name.
  • RFC: If this change has an associated RFC, please link it in the description.
  • ENT PR: If this change has an associated ENT PR, please link it in the
    description. Also, make sure the changelog is in this PR, not in your ENT PR.

PCI review checklist

  • I have documented a clear reason for, and description of, the change I am making.
  • If applicable, I've documented a plan to revert these changes if they require more than reverting the pull request.
  • If applicable, I've documented the impact of any changes to security controls.

Examples of changes to security controls include using new access control methods, adding or removing logging pipelines, etc.

@github-actions github-actions Bot added the hashicorp-contributed-pr If the PR is HashiCorp (i.e. not-community) contributed label Jul 14, 2025
@github-actions

github-actions Bot commented Jul 14, 2025

Copy link
Copy Markdown

CI Results:
All Go tests succeeded! ✅

@elliesterner
elliesterner marked this pull request as ready for review July 14, 2025 16:45
@elliesterner
elliesterner requested review from a team as code owners July 14, 2025 16:45
@elliesterner
elliesterner requested a review from claudiac-m July 14, 2025 16:45
@elliesterner elliesterner changed the title Adding MetricsReporter interface so that databaseBackend's can share their connection counts with CensusManager Add MetricsReporter interface so that databaseBackend's can share their connection counts with CensusManager Jul 14, 2025
@github-actions

github-actions Bot commented Jul 14, 2025

Copy link
Copy Markdown

Build Results:
All builds succeeded! ✅

@elliesterner elliesterner added this to the 1.20.1 milestone Jul 14, 2025
@elliesterner elliesterner modified the milestones: 1.20.1, 1.21.0-rc1 Jul 14, 2025
@elliesterner
elliesterner requested a review from Copilot July 14, 2025 18:25

Copilot AI left a comment

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.

Pull Request Overview

This PR introduces a new MetricsReporter interface and implements it in the database backend to expose connection counts, adds constants for supported database plugins, and covers the new functionality with unit tests.

  • Define MetricsReporter in sdk/logical/logical.go
  • Implement GetConnectionMetrics in builtin/logical/database/backend.go
  • Add database plugin identifiers in helper/pluginconsts/plugin_consts.go
  • Add tests for connection‐count reporting and Cassandra multiplexing in builtin/logical/database/backend_test.go

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
sdk/logical/logical.go Introduce MetricsReporter interface
helper/pluginconsts/plugin_consts.go Add constants for supported database plugin names
builtin/logical/database/backend.go Implement GetConnectionMetrics on the backend
builtin/logical/database/backend_test.go Add tests for GetConnectionMetrics and Cassandra
Comments suppressed due to low confidence (4)

sdk/logical/logical.go:206

  • [nitpick] The comment says "returns a metric" but the method returns multiple metrics; consider updating it to "returns metrics" for clarity.
// MetricsReporter is an optional interface that returns a

helper/pluginconsts/plugin_consts.go:29

  • [nitpick] The acronym "DB" is typically capitalized in Go; consider renaming constants from Db* to DB* for consistency with Go conventions.
	DbCassandraPlugin        = "cassandra"

helper/pluginconsts/plugin_consts.go:36

  • [nitpick] The constant name suggests PostgreSQL but the value is "pgx"; consider aligning the name and value or adding a comment to explain the mapping.
	DbPostgresqlPlugin       = "pgx"

builtin/logical/database/backend.go:510

  • [nitpick] It would be helpful to clarify in the comment that the returned map keys are plugin type strings and values are connection counts.
// GetConnectionMetrics returns a count of the active Database connections.

Comment thread builtin/logical/database/backend.go Outdated
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@elliesterner
elliesterner merged commit 23e04c2 into main Jul 14, 2025
90 of 91 checks passed
@elliesterner
elliesterner deleted the VAULT-32655/add-reporter-interface branch July 14, 2025 19:52
Erfankam pushed a commit to Erfankam/vault that referenced this pull request Sep 1, 2025
…ir connection counts with CensusManager (hashicorp#31269)

* add interface and impl

* add tests

* fix comments

* Update builtin/logical/database/backend.go

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

---------

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

hashicorp-contributed-pr If the PR is HashiCorp (i.e. not-community) contributed pr/no-changelog

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants