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

Adding Gitlab to Transports #8246

Merged
merged 13 commits into from Feb 27, 2018

Conversation

Projects
None yet
5 participants
@dword4
Copy link
Contributor

dword4 commented Feb 13, 2018

DO NOT DELETE THIS TEXT

Please note

Please read this information carefully. You can run ./scripts/pre-commit.php to check your code before submitting.

Testers

If you would like to test this pull request then please run: ./scripts/github-apply <pr_id>, i.e ./scripts/github-apply 5926

@CLAassistant

This comment has been minimized.

Copy link

CLAassistant commented Feb 13, 2018

CLA assistant check
All committers have signed the CLA.

dword4 added some commits Feb 13, 2018

@laf
Copy link
Member

laf left a comment

Wow, sometimes our contributors amaze me :)

Thanks for submitting this.

On the recovery side of it, would you not want to close the issue? I'm guessing the issue with that is knowing the issue to close?


## Gitlab

LibreNMS will create issues for warning and critical level alerts however only title and description are set. Uses Peronal access tokens to authenticate with Gitlab and will store the token in cleartext.

This comment has been minimized.

@laf

laf Feb 15, 2018

Member

Personal (SP)

This comment has been minimized.

@dword4

dword4 Feb 15, 2018

Contributor

Yeah unless LibreNMS stores the Issue ID somewhere you end up having to do a search through issues to match the alert to the existing issue. I'm sure it can be done but since I based this off the Jira transport I don't believe it is built to resolve the issues yet, however that definitely merits investigation so that run-away recycling alerts don't just pile up.

This comment has been minimized.

@dword4

dword4 Feb 18, 2018

Contributor

I almost have auto-resolution worked out, however it seems like the code block below doesn't seem to actually work when I tested it

        // Don't create tickets for resolutions
        if ($obj['severity'] == 'recovery' && $obj['msg'] != 'This is a test alert') {
            return true;
        }

In my testing setup I get tickets created on initial alert and when the alert clears. Once I get this nailed down issues should be auto-closed when the alert clears however this brings me to another question. Should this be a configurable behavior or just the default behavior for the Gitlab transport?

This comment has been minimized.

@laf

laf Feb 18, 2018

Member

Isn't that just to stop a new ticket being created for recovery?

This comment has been minimized.

@dword4

dword4 Feb 18, 2018

Contributor

The code block I pasted is supposed to do that, however in testing it doesn't appear to work.

This comment has been minimized.

@laf

laf Feb 19, 2018

Member

If you just do a var_dump($obj);exit; and then run alerts.php manually to see what data it has that might help work out why.

This comment has been minimized.

@dword4

dword4 Feb 23, 2018

Contributor

Okay I figured it out after putting some time in, $obj['severity'] never changes to recovery when say a device comes back online but $obj['state'] changes from 1 to 0 so that code would never work. Should I bother with trying to flesh out a somewhat-shaky auto-resolve system to clear out issues once it clears or do you think this is sufficient for now?

This comment has been minimized.

@laf

laf Feb 27, 2018

Member

I think it's ok for now.

What I was thinking was having another column in the alerts table to store a serialized array of data you can store which would include anything back from the initial call and then when a recover comes through, you can access that data and do something else. I.e save the original issue number created - which I assume is returned in the original api call?

dword4 and others added some commits Feb 23, 2018

@laf laf added the Schema label Feb 27, 2018

@laf

laf approved these changes Feb 27, 2018

@scrutinizer-notifier

This comment has been minimized.

Copy link

scrutinizer-notifier commented Feb 27, 2018

The inspection completed: 2 updated code elements

@laf laf merged commit 764abde into librenms:master Feb 27, 2018

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
license/cla Contributor License Agreement is signed.
Details

inetAnt added a commit to criteo-forks/librenms that referenced this pull request Mar 19, 2018

alerting: Added Gitlab transport for alerting (librenms#8246)
* gitlab alert transport added

* fixed SQL file and cleaned up a bit

* updated license information

* added relevant information Transports documentation

* Fixed spacing issues

* removed unnecessary text from variable

* fixed sql schema file syntax

* changed sql schema order

* fixed creation of issues for resolution events

* addressed file name conflict

* fixed spacing, again

* removed unused variable

* Update Transports.md
@lock

This comment has been minimized.

Copy link

lock bot commented May 16, 2018

This thread has been automatically locked since there has not been any recent activity after it was closed.

@lock lock bot locked as resolved and limited conversation to collaborators May 16, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.