Problem/Motivation

src/Plugin/ElasticSearch/Connector/StandardConnector.php logs all events. This is good for development, but unnecessary for production.

See #3371048: Logging is too noisy by default

Proposed resolution

Stop logging network traffic all the time: add an option (with appropriate warnings) so it only logs when an admin requests it.

Remaining tasks

  1. Write a patch
  2. Review and feedback
  3. RTBC and feedback
  4. Commit
  5. Release — released in 8.0.0-alpha1

User interface changes

  1. Adds a Enable debugging mode: log ElasticSearch network traffic checkbox to the Standard and Basic Authentication server types.

API changes

None.

Data model changes

  1. Adds a enable_debug_logging key to the plugin.plugin_configuration.elasticsearch_connector.standard configuration.
  2. Note that plugin.plugin_configuration.elasticsearch_connector.basicauth extends plugin.plugin_configuration.elasticsearch_connector.standard configuration, so enable_debug_logging automatically gets added to plugin.plugin_configuration.elasticsearch_connector.basicauth.

Issue fork elasticsearch_connector-3427092

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

sokru created an issue. See original summary.

sokru’s picture

sokru commented
Issue summary: View changes

mparker17 made their first commit to this issue’s fork.

mparker17’s picture

mparker17
he/him
Primary language English
Location UTC-5
commented
Assigned: Unassigned » mparker17

I have been working on this, and have working code, so I'm going to assign this to myself.

mparker17 credited sukhi.chuhan.

mparker17’s picture

mparker17
he/him
Primary language English
Location UTC-5
commented

Crediting @sukhi.chuhan for User Interface / User Experience help.

mparker17 opened merge request !43

mparker17’s picture

mparker17
he/him
Primary language English
Location UTC-5
commented
Assigned: mparker17 » Unassigned
Status: Active » Needs review

This merge request is ready for review. Sorry for the delay, I was having some local issues with PHPUnit. Feedback is welcome.

Thanks again to @sukhi.chuhan!

mparker17’s picture

mparker17
he/him
Primary language English
Location UTC-5
commented
Status: Needs review » Reviewed & tested by the community
Parent issue: » #3427243: Plan for 8.0.0-alpha release

Adding #3427243: Plan for 8.0.0-alpha release as a parent issue.

Also going to move this to RTBC for another maintainer to take a look at.

sokru’s picture

sokru commented
Status: Reviewed & tested by the community » Needs work

Thanks, looks good! I'd suggest we remove the "temp" and "temporary" terminology from MR. The word "debug" itself should indicate that its not meant to be used on production.

mparker17’s picture

mparker17
he/him
Primary language English
Location UTC-5
commented

@sokru, thanks for the review!

@sukhi.chuhan, the UX design lead that I consulted with for the setting's text, advocated for using "temporary" to indicate that the setting should only be used for the short term, but she also okay with removing it if the word "temporary" feels duplicative for the audience (i.e.: admins changing ElasticSearch server settings).

I will push a commit shortly to remove it

mparker17’s picture

mparker17
he/him
Primary language English
Location UTC-5
commented
Status: Needs work » Reviewed & tested by the community

Made all the requested changes, and fixed the PHPStan errors, so moving back to "RTBC"

  • sokru committed daa55ff9 on 8.0.x authored by mparker17 
    Issue #3427092 by mparker17, sokru, sukhi.chuhan: Logging should be...
sokru’s picture

sokru commented
Status: Reviewed & tested by the community » Fixed

Thanks!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

mparker17’s picture

mparker17
he/him
Primary language English
Location UTC-5
commented
Issue summary: View changes

Update the issue summary to document changes made in this ticket.