Skip to content

[couchbase] Make changes to separate username/password from host #10490

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Aug 5, 2024
Merged

[couchbase] Make changes to separate username/password from host #10490

merged 8 commits into from
Aug 5, 2024

Conversation

devamanv
Copy link
Contributor

@devamanv devamanv commented Jul 15, 2024

Proposed commit message

The PR contains changes to separate username/password from the host string and also mark the password field as secret.

Important points for the reviewers:

  • The changes will continue to work for the existing users and the old Host string pattern containing username and password
  • The changes were tested with Couchbase v7.1 and sync-gateway v2.8.0-enterprise

Checklist

  • I have reviewed tips for building integrations and this pull request is aligned with them.
  • I have verified that all data streams collect metrics or logs.
  • I have added an entry to my package's changelog.yml file.
  • I have verified that Kibana version constraints are current according to guidelines.

Author's Checklist

  • Mark the password field as secret
  • Upgrading the package doesn't break the existing integration and metrics still come through
  • No dashboards are broken

How to test this PR locally

  • Spin up the Elastic stack and point your elastic-package to the running Kibana instance
  • Build the package locally and install the package using elastic-package install --zip couchbase-1.8.0.zip
  • Upgrade the integration to the latest version i.e. 1.8.0, there shouldn't be any errors. If you find any errors while upgrading, please report it by adding a comment to this PR.
  • You should now see separate fields for username and password under advanced options while configuring the Integration for metrics collection
  • Take a look at the dashboards, they shouldn't be broken and metrics should still be coming in just fine

Related issues

Screenshots

image
Metrics Overview Dashboard

image
Node Overview Dashboard

image
Sync Gateway Overview Dashboard

@devamanv devamanv added the enhancement New feature or request label Jul 15, 2024
@elasticmachine
Copy link

elasticmachine commented Jul 15, 2024

🚀 Benchmarks report

To see the full report comment with /test benchmark fullreport

@andrewkroh andrewkroh added Integration:couchbase Couchbase Team:Obs-InfraObs Observability Infrastructure Monitoring team [elastic/obs-infraobs-integrations] labels Jul 19, 2024
@devamanv devamanv marked this pull request as ready for review July 24, 2024 07:51
@devamanv devamanv requested a review from a team as a code owner July 24, 2024 07:51
Copy link
Contributor

@ali786XI ali786XI left a comment

Choose a reason for hiding this comment

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

Can you check there may be one more data stream cache which is not included here in this PR?

@ali786XI
Copy link
Contributor

@devamanv Also, I wanted to propose one thing. It will be good to test these changes by including them in the system tests files which are using currently the http://Administrator:password@{{Hostname}}:{{Port[0]}} format. We can instead provide the username and password separately. This way we can ensure nothing is failing.

@harnish-crest-data harnish-crest-data self-requested a review July 25, 2024 06:23
Copy link
Contributor

@harnish-crest-data harnish-crest-data left a comment

Choose a reason for hiding this comment

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

LGTM!

@harnish-crest-data
Copy link
Contributor

@devamanv Also, I wanted to propose one thing. It will be good to test these changes by including them in the system tests files which are using currently the http://Administrator:password@{{Hostname}}:{{Port[0]}} format. We can instead provide the username and password separately. This way we can ensure nothing is failing.

+1

@elasticmachine
Copy link

💚 Build Succeeded

History

Copy link

@devamanv devamanv merged commit d455c79 into elastic:main Aug 5, 2024
5 checks passed
@devamanv devamanv deleted the enable-secrets-couchbase branch August 5, 2024 06:21
@elasticmachine
Copy link

Package couchbase - 1.8.0 containing this change is available at https://epr.elastic.co/search?package=couchbase

harnish-crest-data pushed a commit to chavdaharnish/integrations that referenced this pull request Feb 4, 2025
harnish-crest-data pushed a commit to chavdaharnish/integrations that referenced this pull request Feb 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request Integration:couchbase Couchbase Team:Obs-InfraObs Observability Infrastructure Monitoring team [elastic/obs-infraobs-integrations]
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants