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.

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

[dev.icinga.com #11944] Icinga should log HTTP bodies for API requests #4282

Closed
icinga-migration opened this issue Jun 15, 2016 · 13 comments · Fixed by #6657
Closed

[dev.icinga.com #11944] Icinga should log HTTP bodies for API requests #4282

icinga-migration opened this issue Jun 15, 2016 · 13 comments · Fixed by #6657
Assignees
Labels
area/api REST API area/log Logging related enhancement New feature or request
Milestone

Comments

@icinga-migration
Copy link

This issue has been migrated from Redmine: https://dev.icinga.com/issues/11944

Created by tclh123 on 2016-06-15 09:05:50 +00:00

Assignee: (none)
Status: New
Target Version: (none)
Last Update: 2016-08-05 06:39:22 +00:00 (in Redmine)

Backport?: Not yet backported
Include in Changelog: 1

Should icinga provide access log whiich logging api requests?


Relations:

@icinga-migration
Copy link
Author

Updated by mfriedrich on 2016-06-15 09:08:54 +00:00

  • Status changed from New to Feedback
  • Assigned to set to tclh123

Look into icinga2.log - what's missing?

@icinga-migration
Copy link
Author

Updated by tclh123 on 2016-06-15 09:20:02 +00:00

dnsmichi wrote:

Look into icinga2.log - what's missing?

Oh, I see `HttpServerConnection`. flood by other information and I didn't notice it.
Thank you!

@icinga-migration
Copy link
Author

Updated by tclh123 on 2016-06-15 09:24:18 +00:00

Can I make it contain HTTP params through some config change?

@icinga-migration
Copy link
Author

Updated by mfriedrich on 2016-06-15 09:48:09 +00:00

What exactly do you mean?

@icinga-migration
Copy link
Author

Updated by tclh123 on 2016-06-15 10:08:57 +00:00

dnsmichi wrote:

What exactly do you mean?

For example, when I run

```
$ curl -k -s -u 'root:icinga' -H 'X-HTTP-Method-Override: GET' -X POST 'https://localhost:5665/v1/objects/hosts' -d '{ filter "host.vars.os == os", filter_vars { os "Linux" } }'
```

The log will only record like, which is useless

```
[xxx] information/HttpServerConnection: Request: GET /v1/objects/hosts (root)
```

What I want is,

```
[xxx] information/HttpServerConnection: Request: GET /v1/objects/hosts (root) { filter "host.vars.os == os", filter_vars { os "Linux" } }
```

Which records the post data.

@icinga-migration
Copy link
Author

Updated by gbeutner on 2016-08-05 06:39:10 +00:00

  • Status changed from Feedback to New
  • Assigned to deleted tclh123

@icinga-migration
Copy link
Author

Updated by gbeutner on 2016-08-05 06:39:22 +00:00

  • Subject changed from Any api access log? to Icinga should log HTTP bodies for API requests

@icinga-migration
Copy link
Author

Updated by gbeutner on 2016-09-02 07:42:32 +00:00

  • Relates set to 12625

@icinga-migration icinga-migration added enhancement New feature or request area/api REST API labels Jan 17, 2017
@dnsmichi dnsmichi added wishlist area/log Logging related labels Feb 7, 2017
@dnsmichi
Copy link
Contributor

dnsmichi commented Feb 8, 2017

HttpUtility::FetchRequestParameters() contains the request body logging already, but only for debug builds. Could become spam-ish if we enable that for the LogInformation level.

@dnsmichi dnsmichi added the help wanted Extra attention is needed label Apr 26, 2017
@Crunsher
Copy link
Contributor

Is this something we really want? I'm itching to close this

@ClemensBW
Copy link

yes, for development or logging what other api users reqest with his access. Enable the icinga2 debug option is very chatty in big stages.

@dnsmichi
Copy link
Contributor

That really depends on more advanced log filters.

Then you may have the chance to add specific logger objects which filter only API requests on a defined level. I wouldn't go the route to just add and log it to the debug log now. The next feature request is probably to log entire cluster messages for debugging.

That also may decrease the overall performance in large setups with many requests and messages.

@dnsmichi dnsmichi added needs-sponsoring Not low on priority but also not scheduled soon without any incentive and removed help wanted Extra attention is needed wishlist labels Jan 3, 2018
@dnsmichi dnsmichi removed the needs-sponsoring Not low on priority but also not scheduled soon without any incentive label Oct 9, 2018
@dnsmichi
Copy link
Contributor

dnsmichi commented Oct 9, 2018

Actually I don't care much anymore whether the debug log is flooded or not. The patch just removes the debug build ifdefs, I always had that enabled here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/api REST API area/log Logging related enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants