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

new plugin: nginx_plus #3214

Merged
merged 14 commits into from
Sep 19, 2017
Merged

new plugin: nginx_plus #3214

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
0f772a3
Nginx Plus stats
mplonka Sep 9, 2017
02eb8cd
applying code review suggestions
mplonka Jul 20, 2017
74374cb
fixing test data
mplonka Jul 21, 2017
079ff75
stream session stats reuse struct for http response stats
mplonka Jul 21, 2017
579952e
untangle nginx_plus from nginx
poblahblahblah Sep 9, 2017
5987e64
do not use errchan
poblahblahblah Sep 9, 2017
c95dad2
Updating README with correct measurements, fields, and tags
poblahblahblah Sep 9, 2017
6bfc295
Missed a few measurements
poblahblahblah Sep 9, 2017
f3d0931
renaming measurements from nginx_ to nginx_plus_
poblahblahblah Sep 11, 2017
f965bca
Moving Client to NginxPlus Struct; Adding timeout
poblahblahblah Sep 13, 2017
767fd53
Initialize client only once
poblahblahblah Sep 14, 2017
33fe1f0
fix two pointer dereference issues
poblahblahblah Sep 14, 2017
c84861a
Check pointers for nil before dereferencing
poblahblahblah Sep 15, 2017
b19a1da
Updating output example in README.md
poblahblahblah Sep 19, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions plugins/inputs/all/all.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ import (
_ "github.com/influxdata/telegraf/plugins/inputs/nats_consumer"
_ "github.com/influxdata/telegraf/plugins/inputs/net_response"
_ "github.com/influxdata/telegraf/plugins/inputs/nginx"
_ "github.com/influxdata/telegraf/plugins/inputs/nginx_plus"
_ "github.com/influxdata/telegraf/plugins/inputs/nsq"
_ "github.com/influxdata/telegraf/plugins/inputs/nsq_consumer"
_ "github.com/influxdata/telegraf/plugins/inputs/nstat"
Expand Down
123 changes: 123 additions & 0 deletions plugins/inputs/nginx_plus/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,123 @@
# Telegraf Plugin: nginx_plus

### Configuration:

```
# Read Nginx Plus' advanced status information
[[inputs.nginx_plus]]
## An array of Nginx status URIs to gather stats.
urls = ["http://localhost/status"]
```

### Measurements & Fields:

- nginx_plus_processes
- respawned
- nginx_plus_connections
- accepted
- dropped
- active
- idle
- nginx_plus_ssl
- handshakes
- handshakes_failed
- session_reuses
- nginx_plus_requests
- total
- current
- nginx_plus_upstream, nginx_plus_stream_upstream
- keepalive
- zombies
- nginx_plus_upstream_peer, nginx_plus_stream_upstream_peer
- requests
- unavail
- healthchecks_checks
- header_time
- response_time
- state
- active
- downstart
- healthchecks_last_passed
- weight
- responses_1xx
- responses_2xx
- responses_3xx
- responses_4xx
- responses_5xx
- received
- selected
- healthchecks_fails
- healthchecks_unhealthy
- backup
- responses_total
- sent
- fails
- downtime


### Tags:

- nginx_plus_processes, nginx_plus_connections, nginx_plus_ssl, nginx_plus_requests
- server
- port
- host

- nginx_plus_upstream, nginx_plus_stream_upstream
- host
- upstream
- server
- port

- nginx_plus_upstream_peer, nginx_plus_stream_upstream_peer
- id
- host
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't list host since it is added globally by Telegraf (depending on config).

- upstream
- server
- port
- serverAddress
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be server_address to match the naming style. This is the address of the upstream? If so maybe it would be more descriptive to call it upstream_address?


### Example Output:

Using this configuration:
```
[[inputs.nginx_plus]]
## An array of Nginx Plus status URIs to gather stats.
urls = ["http://localhost/status"]
```

When run with:
```
./telegraf -config telegraf.conf -input-filter nginx_plus -test
```

It produces:
```
* Plugin: inputs.nginx_plus, Collection 1
> nginx_plus_processes,server=localhost,port=12021,host=word.local respawned="0xc420075538" 1504922954000000000
Copy link
Contributor

Choose a reason for hiding this comment

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

respawned is an address, this is probably a bug where we need to dereference a pointer.

Copy link
Contributor Author

@poblahblahblah poblahblahblah Sep 14, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

@danielnelson I think I fixed all of these. Here's the commit: 33fe1f0 - It looks like in the original commits the test was written like this, which was making the tests pass:

                       "selected": func() *int64 {
                               var v int64 = 1451606400000
                               return &v
                       }(),

I updated the test to just expect the int64:

                       "selected":               int64(1451606400000),

And updated the map:

"selected":               *peer.Selected,

Those were the only two instances where this was happening. I made the changes to both nginx_plus.go and the tests, and made sure this compiled. I then ran telegraf in test mode against one of our load balancers and I did not see any addresses in the output.

If this looks like a good fix I will update the README to remove the addresses from the sample output.

Copy link
Contributor

Choose a reason for hiding this comment

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

You will want to check that they are not nil before dereferencing, I assume they are pointers because in some versions they don't exist.

Copy link
Contributor Author

@poblahblahblah poblahblahblah Sep 14, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

How would you recommend I do that? Something like this for respawned?

func (s *Status) gatherProcessesMetrics(tags map[string]string, acc telegraf.Accumulator) {
    var respawned int

    if s.Processes.Respawned != nil {
      respawned = *s.Processes.Respawned
    }

    acc.AddFields(
        "nginx_plus_processes",
        map[string]interface{}{
            "respawned": respawned,
        },
        tags,
    )

}

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Pushed a fix in c84861a

> nginx_plus_connections,server=localhost,port=12021,host=word.local accepted=4458727685i,dropped=10138424i,active=10256i,idle=29390i 1504922954000000000
> nginx_plus_ssl,server=localhost,port=12021,host=word.local handshakes=0i,handshakes_failed=0i,session_reuses=0i 1504922954000000000
> nginx_plus_requests,host=word.local,server=localhost,port=12021 total=147885504244i,current=10019i 1504922954000000000
> nginx_plus_upstream,host=word.local,upstream=dataserver80,server=localhost,port=12021 zombies=0i,keepalive=0i 1504922954000000000
> nginx_plus_upstream_peer,id=0,server=localhost,port=12021,host=word.local,upstream=dataserver80,serverAddress=10.10.102.181:80 responses_5xx=27831i,healthchecks_unhealthy=1i,downtime=484817i,healthchecks_last_passed=true,responses_1xx=0i,active=22i,requests=2620930i,responses_total=2620652i,fails=4i,downstart=0i,state="up",responses_4xx=16i,healthchecks_checks=14133i,selected="0xc4201b22e8",response_time=95i,responses_2xx=2592805i,weight=1i,responses_3xx=0i,sent=3802831967i,received=536695496i,unavail=4i,healthchecks_fails=27i,header_time=94i,backup=false 1504922954000000000
Copy link
Contributor

Choose a reason for hiding this comment

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

selected also has an address and probably needs dereferenced. Double check that other similar pointers are dereferenced.

```

### Reference material

Structures for Nginx Plus have been built based on history of
[status module documentation](http://nginx.org/en/docs/http/ngx_http_status_module.html)
Copy link
Contributor

Choose a reason for hiding this comment

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

Lets move this up to the first section, and mention that you need a commercial license to have access to this module.


Subsequent versions of status response structure available here:

- [version 1](http://web.archive.org/web/20130805111222/http://nginx.org/en/docs/http/ngx_http_status_module.html)

- [version 2](http://web.archive.org/web/20131218101504/http://nginx.org/en/docs/http/ngx_http_status_module.html)

- version 3 - not available

- [version 4](http://web.archive.org/web/20141218170938/http://nginx.org/en/docs/http/ngx_http_status_module.html)

- [version 5](http://web.archive.org/web/20150414043916/http://nginx.org/en/docs/http/ngx_http_status_module.html)

- [version 6](http://web.archive.org/web/20150918163811/http://nginx.org/en/docs/http/ngx_http_status_module.html)

- [version 7](http://web.archive.org/web/20161107221028/http://nginx.org/en/docs/http/ngx_http_status_module.html)
Loading