search

cloudfoundry-community / firehose-to-syslog

Send firehose events from Cloud Foundry to syslog.
MIT License
44 stars 58 forks source link

Add org filter #187

Closed datianshi closed 6 years ago

datianshi commented 6 years ago

Add a way to include log message in certain logs … extra tag can add cf_orgs:org1|org2|org3 create more tests on eventRouting_test.go for more coverage

shinji62 commented 6 years ago

I prefer that you create another flag, hijacking the extraFields one. extraFields is for one purpose, adding some extra fields to the output logs.

Quite confusing to use same field for another purpose.

datianshi commented 6 years ago

@shinji62 took your feedback and added a new flag

Thanks

datianshi commented 6 years ago

also did a squash @shinji62

cloutierf commented 6 years ago

Here the environment variable appear to be for white listing orgs but what we need is to be able to exclude some orgs.

A think a less confusing name could be: EXCLUDE_ORGS = followed a comma separated list of orgs

To be consistent with the EVENTS environment variable, the separator should be a comma instead of a |.

cloutierf commented 6 years ago

We might need to have both a include_orgs and exclude_orgs variable to support both use case. Default for include_orgs when not specified should be all orgs. Default for exclude_orgs when not specified should be empty (no orgs excluded)

On Thursday, May 31, 2018, Shaozhen Ding notifications@github.com wrote:

@datianshi commented on this pull request.

In eventRouting/event_filter.go https://github.com/cloudfoundry-community/firehose-to-syslog/pull/187#discussion_r192253761 :

  • "strings"
  • fevents "github.com/cloudfoundry-community/firehose-to-syslog/events" +)

  • +//EventFilter Given an Event Filter out unwanted event +type EventFilter func(*fevents.Event) bool

  • +//HasIgnoreField Filter out the event has ignored app filed +func HasIgnoreField(event *fevents.Event) bool {

  • ignored, hasIgnoredField := event.Fields["cf_ignored_app"]
  • return ignored == true && hasIgnoredField +}

  • +//NotInCertainOrgs Filter out events not in certain orgs +func NotInCertainOrgs(orgFilters string) EventFilter {

using comma instead of | in the new commit

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/cloudfoundry-community/firehose-to-syslog/pull/187#discussion_r192253761, or mute the thread https://github.com/notifications/unsubscribe-auth/AX-PoKW3_c2te-IeH2lfa82VV-OoGZ2Nks5t4GwRgaJpZM4UUPUy .

datianshi commented 6 years ago

@cloutierf I believe you are that client... this is an opensource community that serve community need. Please refine your specific requirements in internal tracker... this particular pull request is not about exclude_orgs

shinji62 commented 6 years ago

@datianshi Small change and we should be good to go.

datianshi commented 6 years ago

@shinji62 made the change on the flag

datianshi commented 6 years ago

It will not take any change on system metric. This is for LogMessage

On Jun 1, 2018, at 1:45 AM, Etourneau Gwenn notifications@github.com wrote:

@shinji62 commented on this pull request.

In eventRouting/event_filter.go:

  • fevents "github.com/cloudfoundry-community/firehose-to-syslog/events" +)

  • +//EventFilter Given an Event Filter out unwanted event +type EventFilter func(*fevents.Event) bool

  • +//HasIgnoreField Filter out the event has ignored app filed +func HasIgnoreField(event *fevents.Event) bool {

  • ignored, hasIgnoredField := event.Fields["cf_ignored_app"]
  • return ignored == true && hasIgnoredField +}

  • +//NotInCertainOrgs Filter out events not in certain orgs +func NotInCertainOrgs(orgFilters string) EventFilter {

  • return func(event *fevents.Event) bool {
  • orgName := event.Fields["cf_org_name"] What's happen if the field "cf_org_name" do not exist in the event ? Like for Counter, Error type of message?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.