Skip to content
This repository has been archived by the owner on May 4, 2022. It is now read-only.

Ceilometer metrics support #86

Merged
merged 14 commits into from
Jul 10, 2020
Merged

Ceilometer metrics support #86

merged 14 commits into from
Jul 10, 2020

Conversation

paramite
Copy link
Member

@paramite paramite commented Jul 7, 2020

This PR makes SG to understand Ceilometer metrics

@paramite paramite changed the title Mmagr amqp10connections Ceilometer metrics support Jul 7, 2020
@leifmadsen leifmadsen self-requested a review July 7, 2020 23:20
Copy link
Member

@leifmadsen leifmadsen left a comment

Choose a reason for hiding this comment

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

Passes CI now. Thanks!

@@ -21,11 +21,11 @@ before_install:
- docker run -p 9200:9200 --name elastic -p 9300:9300 -e "discovery.type=single-node" -d docker.elastic.co/elasticsearch/elasticsearch:$ELASTIC_VERSION
- docker pull quay.io/interconnectedcloud/qdrouterd:$QDROUTERD_VERSION
- docker run -p 5672:5672 -d quay.io/interconnectedcloud/qdrouterd:$QDROUTERD_VERSION
- docker pull centos:7
- docker pull centos:8
Copy link
Member

Choose a reason for hiding this comment

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

👍

Copy link
Contributor

@csibbitt csibbitt left a comment

Choose a reason for hiding this comment

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

Go!

yum remove -y git*
yum install -y git216-all
yum install -y golang qpid-proton-c-devel iproute
# below is not available currently
Copy link
Contributor

Choose a reason for hiding this comment

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

Can these just be removed, or a note added explaining when they would be uncommented?

@@ -197,11 +191,10 @@ func (cs *CacheServer) Put(incomingData incoming.MetricDataFormat) {
case buffer = <-freeList:
//go one from buffer
default:
buffer = new(IncomingBuffer)
buffer = &IncomingBuffer{}
Copy link
Contributor

Choose a reason for hiding this comment

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

For my education, is there a reason other than style to prefer this version?

if dataInterface.ISNew() {
dataInterface.SetNew(false)
for index := range dataInterface.GetValues() {
m, err := tsdb.NewPrometheusMetric(usetimestamp, dataInterface.GetDataSourceName(), dataInterface, index)
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

}

//GetInterval ...
//GetInterval returns hardcoded defaultCeilometerInterval, because Ceilometer metricDesc
//does not contain interval information (are not periodically sent at all) and any reasonable
Copy link
Contributor

Choose a reason for hiding this comment

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

👍 Thanks for making this explicit in the comments!

//MetricHandlerHTML contains HTML for default endpoint
const MetricHandlerHTML = `
<html>
<head><title>Collectd Exporter</title></head>
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably should s/Collectd/Metrics/ in this block, or have it actually driven by the datasource name

@@ -119,7 +123,7 @@ func TestCollectdIncoming(t *testing.T) {
assert.Contains(t, labels, sample.Plugin)
assert.Contains(t, labels, "instance")
// test GetMetricDesc behaviour
metricDesc := "Service Assurance exporter: 'pluginname' Type: 'collectd' Dstype: 'gauge' Dsname: 'value1'"
metricDesc := "Service Telemetry exporter: 'pluginname' Type: 'collectd' Dstype: 'gauge' Dsname: 'value1'"
Copy link
Contributor

Choose a reason for hiding this comment

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

😄 Deep cleaning

@leifmadsen leifmadsen merged commit c27d3e0 into master Jul 10, 2020
@leifmadsen leifmadsen deleted the mmagr-amqp10connections branch July 10, 2020 18:39
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants