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

App events #179

Closed
wants to merge 2 commits into from
Closed

Conversation

jmcarp
Copy link
Member

@jmcarp jmcarp commented Aug 5, 2016

This patch restores filters for app events. There are still fields in the app index patterns, dashboards, visualizations, etc., in this repo that use app events, so I'm not sure why the filter was dropped. The relevant commit is e2c7a2a, so @cromega might have some insight into that decision. Anyway, we'd like to keep the app events dashboard, so I figured I'd propose restoring these filters.

cc @LinuxBozo

@hannayurkevich
Copy link
Collaborator

Hi @jmcarp,

The app events parsing rules were dropped some time ago (as well as other filters) because they were outdated and not unit tested.

After that we restored some removed parsing rules, reworked them and unit tested. App event parsing rules were not restored yet but we have plans to to that (as well as add parsing rules for other app log types). There is an open issue concerning this - #158.

As soon as you rely on this functionality I'll look into the issue as soon as I finish my current task (my current task relates to filters and tests refactoring, so I better finish it before adding new parsing rules). If this doesn't work for you, then I can review your PR and merge the changes, but please make sure to make the filtering rules up-to-date and add UTs and ITs for it first.

Thanks!

@jmcarp
Copy link
Member Author

jmcarp commented Aug 5, 2016

That sounds great--I definitely don't want to create extra work by restoring filters before you're finished refactoring. I would also be happy to help restore app events after the refactor is done if that would be useful. By the way, we're using the old app events filters now, and they seem to work fine--was the problem that those filters weren't well tested?

@hannayurkevich
Copy link
Collaborator

@jmcarp, answering to your question, the filters are both outdated and also not unit-tested. There are several issues that makes the filters outdated (incorrect fields names e.g. [@source][app][id], incorrect date filter etc.). Anyway the snippet requires rework, so it is better to do it thoroughly.

I'm planning to start #158 today so the problem will be fixed soon.

@hannayurkevich
Copy link
Collaborator

@jmcarp, I've started looking into issue #158 and found out that actually AppEvent event type is not supported and here is the list of possible event types - https://github.com/cloudfoundry-community/logsearch-for-cloudfoundry/blob/v200.0.0/jobs/ingestor_cloudfoundry-firehose/spec#L35.

How do you get events of this type in your ELK?

@jmcarp
Copy link
Member Author

jmcarp commented Aug 9, 2016

@hannayurkevich: app events aren't streamed through the firehose, so we have to do a little extra work to get them. There's a separate CF app that collects app events from the CF app and prints them to stdout, which allows the firehose to capture them and forward them to logsearch. The original source for the event forwarder is at https://github.com/stayup-io/cf-app-events-logger, but our team has fixed a few issues on that repo, so we're using our fork at https://github.com/18F/cf-app-events-logger instead. I added a quick note about this to the README in this patch: https://github.com/cloudfoundry-community/logsearch-for-cloudfoundry/pull/179/files#diff-04c6e90faac2675aa89e2176d2eec7d8R78

@hannayurkevich
Copy link
Collaborator

cc @Infra-Red, @axelaris

@jmcarp, because the tool you mentioned is not part of logsearch-for-cloudfoundry I don't think it's a good idea to include and support parsing rules for these events.

I think this is a good case of using configurable chain of parsing rules. We don't have this feature yet, but it could be really useful to allow users to define which parsing rules they want to apply (or stick to default rules) and specify custom parsing rules to be applied.

In your case you could add custom parsing rules for AppEvent logs and configure parser to use them in addition to default rules.

I've just created #182 for this enhancement.

@jmcarp
Copy link
Member Author

jmcarp commented Aug 9, 2016

Support for custom parsing rules sounds like a useful feature. It looks like there's a related feature in the works for the base logsearch release at logsearch/logsearch-boshrelease#206.

Just to be clear, parsing app events isn't a new feature that my team is proposing to add--it's a feature that was recently removed that we'd like to keep using. There are still kibana dashboards and visualizations that depend on app event logs, too. I believe the app event collector was written by the original team that developed logsearch-for-cloudfoundry, and I'm guessing it was developed specifically for use with this project.

I'll check with my team to see if anybody would miss app events.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants