Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Suggest DEBUG level logging instead for SAML #74661

Merged
merged 6 commits into from
Jul 30, 2021
Merged

Suggest DEBUG level logging instead for SAML #74661

merged 6 commits into from
Jul 30, 2021

Conversation

ppf2
Copy link
Member

@ppf2 ppf2 commented Jun 28, 2021

Per @DaveCTurner 's discussion with the security team, we believe that DEBUG logging will be sufficient for most cases for SAML troubleshooting. TRACE is only necessary if we believe there's a bug in the implementation. This PR updates the documentation to suggest DEBUG logging instead of TRACE. Thx!

@ppf2 ppf2 added >docs General docs changes :Security/Authentication Logging in, Usernames/passwords, Realms (Native/LDAP/AD/SAML/PKI/etc) labels Jun 28, 2021
@elasticmachine elasticmachine added Team:Docs Meta label for docs team Team:Security Meta label for security team labels Jun 28, 2021
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-docs (Team:Docs)

@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-security (Team:Security)

@elasticsearchmachine elasticsearchmachine added v8.0.0 external-contributor Pull request authored by a developer outside the Elasticsearch team labels Jun 28, 2021
DaveCTurner
DaveCTurner reviewed Jun 29, 2021
View reviewed changes
Copy link
Contributor

@DaveCTurner DaveCTurner left a comment

Choose a reason for hiding this comment

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

I think we should link to https://www.elastic.co/guide/en/elasticsearch/reference/current/logging.html#configuring-logging-levels to add context about configuring loggers in general too.

ppf2
ppf2 commented Jun 30, 2021
View reviewed changes
x-pack/docs/en/security/troubleshooting.asciidoc Outdated
----------------

See <<configuring-logging-levels,configuring logging levels>> for more information.
Copy link
Member Author

Choose a reason for hiding this comment

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

@DaveCTurner Hope this is the right syntax for proper linking 😄

@DaveCTurner
Copy link
Contributor

@elasticmachine update branch

@tvernum tvernum requested a review from jkakavas June 30, 2021 07:55
jkakavas
jkakavas approved these changes Jun 30, 2021
View reviewed changes
Copy link
Member

@jkakavas jkakavas left a comment

Choose a reason for hiding this comment

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

Added a suggestion for wording but I'll leave this up to @lcawl for the final word. I'll also go through our trace logs and see if there is something that makes sense for us to surface in debug level to ease troubleshooting

x-pack/docs/en/security/troubleshooting.asciidoc Outdated
@@ -698,29 +698,30 @@ the `basic` `authProvider` in {kib}. The process is documented in the

*Logging:*

Very detailed trace logging can be enabled specifically for the SAML realm by
Very detailed debug logging can be enabled specifically for the SAML realm by
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Very detailed debug logging can be enabled specifically for the SAML realm by
If the issue you encounter is not covered in the list above, it might be helpful to enable additional logging for the SAML realm to further troubleshoot the issue. You can enable debug logging by

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for this suggestion @jkakavas -- I incorporated a version of that text into my changes.

@tvernum
Copy link
Contributor

tvernum commented Jul 29, 2021

Is someone taking care of getting this PR merged & backported?

@lcawl / @lockewritesdocs are either of you able to run with it?

@tvernum
Copy link
Contributor

tvernum commented Jul 29, 2021

@elasticmachine update branch

@lockewritesdocs lockewritesdocs self-assigned this Jul 29, 2021
@lockewritesdocs
Copy link
Contributor

Is someone taking care of getting this PR merged & backported?

@lcawl / @lockewritesdocs are either of you able to run with it?

I'll pick this one up -- thanks for the ping @tvernum!

@lockewritesdocs
Copy link
Contributor

@elasticmachine update branch

@lockewritesdocs
Copy link
Contributor

@DaveCTurner, does this change also apply to 7.13 and 7.14? If so, I'll add the labels and backport appropriately.

@lockewritesdocs lockewritesdocs added the auto-backport Automatically create backport pull requests when merged label Jul 29, 2021
@DaveCTurner
Copy link
Contributor

I think so, but I don't keep on top of changes in this area so would like someone from @elastic/es-security to confirm.

@tvernum
Copy link
Contributor

tvernum commented Jul 30, 2021

Yes, let's backport all the way to 7.13

@lockewritesdocs lockewritesdocs merged commit d86abdc into master Jul 30, 2021
@lockewritesdocs lockewritesdocs deleted the ppf2-saml-logger branch July 30, 2021 12:02
elasticsearchmachine pushed a commit to elasticsearchmachine/elasticsearch that referenced this pull request Jul 30, 2021
@ppf2 @elasticmachine
Suggest DEBUG level logging instead for SAML (elastic#74661)
bd2e8e3 
* Suggest DEBUG level logging instead for SAML

* Update troubleshooting.asciidoc

* Incorporate reviewer feedback

Co-authored-by: Elastic Machine <[email protected]>
Co-authored-by: Adam Locke <[email protected]>
@elasticsearchmachine elasticsearchmachine mentioned this pull request Jul 30, 2021
Merged
elasticsearchmachine pushed a commit to elasticsearchmachine/elasticsearch that referenced this pull request Jul 30, 2021
@ppf2 @elasticmachine
Suggest DEBUG level logging instead for SAML (elastic#74661)
796e1b1 
* Suggest DEBUG level logging instead for SAML

* Update troubleshooting.asciidoc

* Incorporate reviewer feedback

Co-authored-by: Elastic Machine <[email protected]>
Co-authored-by: Adam Locke <[email protected]>
@elasticsearchmachine elasticsearchmachine mentioned this pull request Jul 30, 2021
Merged
elasticsearchmachine pushed a commit to elasticsearchmachine/elasticsearch that referenced this pull request Jul 30, 2021
@ppf2 @elasticmachine
Suggest DEBUG level logging instead for SAML (elastic#74661)
0e256b2 
* Suggest DEBUG level logging instead for SAML

* Update troubleshooting.asciidoc

* Incorporate reviewer feedback

Co-authored-by: Elastic Machine <[email protected]>
Co-authored-by: Adam Locke <[email protected]>
@elasticsearchmachine elasticsearchmachine mentioned this pull request Jul 30, 2021
Merged
@elasticsearchmachine
Copy link
Collaborator

💚 Backport successful

Status Branch Result
✅ 7.13
✅ 7.14
✅ 7.x

lockewritesdocs pushed a commit that referenced this pull request Jul 30, 2021
@elasticsearchmachine @ppf2 @elasticmachine
Suggest DEBUG level logging instead for SAML (#74661) (#75876)
b7a3496 
* Suggest DEBUG level logging instead for SAML

* Update troubleshooting.asciidoc

* Incorporate reviewer feedback

Co-authored-by: Elastic Machine <[email protected]>
Co-authored-by: Adam Locke <[email protected]>

Co-authored-by: Pius <[email protected]>
Co-authored-by: Elastic Machine <[email protected]>
Co-authored-by: Adam Locke <[email protected]>
lockewritesdocs pushed a commit that referenced this pull request Jul 30, 2021
@elasticsearchmachine @ppf2 @elasticmachine
Suggest DEBUG level logging instead for SAML (#74661) (#75877)
9ce7623 
* Suggest DEBUG level logging instead for SAML

* Update troubleshooting.asciidoc

* Incorporate reviewer feedback

Co-authored-by: Elastic Machine <[email protected]>
Co-authored-by: Adam Locke <[email protected]>

Co-authored-by: Pius <[email protected]>
Co-authored-by: Elastic Machine <[email protected]>
Co-authored-by: Adam Locke <[email protected]>
lockewritesdocs pushed a commit that referenced this pull request Jul 30, 2021
@elasticsearchmachine @ppf2 @elasticmachine
Suggest DEBUG level logging instead for SAML (#74661) (#75878)
32a6d5b 
* Suggest DEBUG level logging instead for SAML

* Update troubleshooting.asciidoc

* Incorporate reviewer feedback

Co-authored-by: Elastic Machine <[email protected]>
Co-authored-by: Adam Locke <[email protected]>

Co-authored-by: Pius <[email protected]>
Co-authored-by: Elastic Machine <[email protected]>
Co-authored-by: Adam Locke <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lockewritesdocs lockewritesdocs lockewritesdocs left review comments

@DaveCTurner DaveCTurner DaveCTurner left review comments

@jkakavas jkakavas jkakavas approved these changes

Assignees

@lockewritesdocs lockewritesdocs

Labels
auto-backport Automatically create backport pull requests when merged >docs General docs changes external-contributor Pull request authored by a developer outside the Elasticsearch team :Security/Authentication Logging in, Usernames/passwords, Realms (Native/LDAP/AD/SAML/PKI/etc) Team:Docs Meta label for docs team Team:Security Meta label for security team v7.13.4 v7.14.0 v7.15.0 v8.0.0-alpha1
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@ppf2 @elasticmachine @DaveCTurner @tvernum @lockewritesdocs @elasticsearchmachine @jkakavas @mark-vieira