Added a capability for generating ceilometer messages #112
Conversation
|
Hmm, I see both of the integration tests failed. Does that happen often? |
|
The last PR to merge was in March, and the integration tests worked then. They also worked in May on this PR, but then failed two weeks ago. #111 |
generator/amqp_snd_th.c
Outdated
| @@ -156,14 +202,16 @@ static char *build_metric_mesg(app_data_t *app, char *time_buf) { | |||
| static void gen_mesg(pn_rwbytes_t *buf, app_data_t *app, char *time_buf) { | |||
| if (app->logs) { | |||
| buf->start = build_log_mesg(app, time_buf); | |||
| } else if (app->ceilometer) { | |||
| buf->start = build_ceil_mesg(app, time_buf); | |||
| } else { | |||
There was a problem hiding this comment.
Noting the implicit default to collectd
generator/gen.c
Outdated
| app.logs = true; | ||
| case 'g': | ||
| if (!strcmp(optarg, "collectd")) { | ||
| // used by default |
There was a problem hiding this comment.
I won't block on this, but as a point of style, I don't think implicit defaults should propagate like this. I think app.collectd deserves to exist and have it's own explicitly checked branch in gen_mesg() like the others. This facilitates changing defaults in the future because it limits the implicit default to a single location. It also makes it explicit to readers that these three options all follow the same pattern, and that none is special (other than being default).
|
@csibbitt I liked your suggestion, so I implemented it. The reason why I originally programmed it like that was, that when I added rsyslog messages to the generator, I tried to touch the collectd generation as little as possible. So when running the generator like normal, you would generate collectd messages and if you wanted rsyslog messages, you had to specify a special parameter (-l). But I guess now, with all the other changes, it makes sense and it looks a lot better to add and use the app->collectd variable. |
vyzigold
force-pushed
the
jwysogla_add-ceilometer-to-generator
branch
from
acb7605 to
0f06813
Compare
|
I rebased on top of master, which has fixed CI now. If nobody has any concerns, I'll merge tomorrow. |
|
Rebased on latest merged changes to the CI refactoring in order to get this cleared. Thanks! |
Recently I needed a way to generate custom ceilometer messages. So I added a possibility to generate them with the generator.
While doing that I also removed some tab characters and replaced them with spaces. I'm sorry for the confusion