Allow add items to array config via ENV #7323

miry · 2016-09-19T07:58:13Z

Allow to create a new templates or tags configs, if there are no records
in the default config.

Support next cases:

Without config file:

$ INFLUXDB_GRAPHITE_ENABLED=true INFLUXDB_GRAPHITE_TEMPLATES="stats.* .host.measurement.cpu,system.* .host.region.measurement*" influxd config

...
[[graphite]]
  enabled = true
  bind-address = ":2003"
  database = "graphite"
  retention-policy = ""
  protocol = "tcp"
  batch-size = 5000
  batch-pending = 10
  batch-timeout = "1s"
  consistency-level = "one"
  templates = ["stats.* .host.measurement.cpu", "system.* .host.region.measurement*"]
  separator = "."
  udp-read-buffer = 0

....

With specified template rule:

[[graphite]]
  enabled = true
  templates = ["stats-config.* .host.measurement.cpu"]

$ INFLUXDB_GRAPHITE_ENABLED=true INFLUXDB_GRAPHITE_0_TEMPLATES_0="stats-over.* .host.measurement.cpu" influxd config
...
[[graphite]]
  enabled = true
  bind-address = ":2003"
  database = "graphite"
  retention-policy = ""
  protocol = "tcp"
  batch-size = 5000
  batch-pending = 10
  batch-timeout = "1s"
  consistency-level = "one"
  templates = ["stats-over.* .host.measurement.cpu"]
  separator = "."
  udp-read-buffer = 0

....

As you see from the first example INFLUXDB_GRAPHITE_TEMPLATES separate rules by ,.

Fixes: #6943

Required for all non-trivial PRs

Rebased/mergable
Tests pass
CHANGELOG.md updated
Sign CLA (if not already signed)

Required only if applicable

You can erase any checkboxes below this note if they are not applicable to your Pull Request.

Provide example syntax

mention-bot · 2016-09-19T07:58:14Z

@miry, thanks for your PR! By analyzing the annotation information on this pull request, we identified @gunnaraasen and @e-dard to be potential reviewers

miry · 2016-11-22T13:51:09Z

@gunnaraasen and @e-dard can you check this PR?

e-dard · 2016-11-23T14:21:45Z

@jackzampolin does this look like what you were after in #6943 ?

jackzampolin · 2016-11-23T16:37:26Z

@e-dard Yeah that's basically it. Want to make it easy to do graphite parsing by setting templates with ENV. I would also ask @gunnaraasen as he worked on this code too.

gunnaraasen

Thanks @miry! Sorry for taking so long to review this PR. I only had a few small cosmetic changes.

@jwilder, can you take a quick look at this to make sure I didn't miss anything?

gunnaraasen · 2016-11-29T20:55:26Z

cmd/influxd/run/config.go

+	case reflect.Bool:
+		boolValue, err := strconv.ParseBool(value)
+		if err != nil {
+			return fmt.Errorf("failed to apply %v to %v using type %v and value '%v'", prefix, structKey, element.Type().String(), value)



Remove unnecessary newline.

fixed. thank you.

gunnaraasen · 2016-11-29T20:56:02Z

cmd/influxd/run/config.go

+	case reflect.Float32, reflect.Float64:
+		floatValue, err := strconv.ParseFloat(value, element.Type().Bits())
+		if err != nil {
+			return fmt.Errorf("failed to apply %v to %v using type %v and value '%v'", prefix, structKey, element.Type().String(), value)



Remove unnecessary newline.

fixed. thank you.

gunnaraasen · 2016-11-30T00:59:26Z

cmd/influxd/run/config.go

+		}
+		element.SetFloat(floatValue)
+	case reflect.Slice:
+		for j := 0; j < element.Len(); j++ {


Can you add in the comment from the existing code back in here?

If the type is s slice, apply to each using the index as a suffix, e.g. GRAPHITE_0

Added comment. also added comment bellow few lines.

gunnaraasen · 2016-11-30T01:01:28Z

cmd/influxd/run/config.go

-				key = strings.ToUpper(fmt.Sprintf("%s_%s", prefix, configName))
+		}
+		element.SetFloat(floatValue)
+	case reflect.Slice:


This case should also handle reflect.Array. I don't think arrays are used in the configuration at this moment, but might as well cover all the bases.

We have array if I understand correctly. In next example there is templates option:

[[graphite]] protocol = "udp" templates = [ "default.* .template.in.config" ]

And arrays converted as slice.

miry · 2016-12-01T13:23:26Z

@gunnaraasen I have updated code according the comments.

e-dard · 2016-12-01T13:49:51Z

@miry looks good. Can you add an entry to the CHANGELOG and make sure you give yourself credit!

e-dard · 2016-12-01T14:11:40Z

@miry can you also rebase from master and push. I think the test failure is because Circle is testing the head ref rather than the merge result of this PR and master. Not sure why it's doing that. The test failure looks completely unrelated to your changes, and when I pulled down your branch it's pretty far behind master.

miry · 2016-12-01T14:17:45Z

@e-dard sure

miry · 2016-12-05T10:49:04Z

@e-dard updated Changelog and rebased against master.

fdrouet · 2016-12-07T15:54:22Z

awesome fix !! when could we expect to see that in a released version of Influxdb ?

burdandrei · 2016-12-22T15:15:58Z

Great fix! waiting it to be merged!

Allow to create a new templates or tags configs, if there are no records in the default config. Fixes: influxdata#6943

miry · 2016-12-23T08:22:29Z

@e-dard @gunnaraasen @jackzampolin I have rebased against master.

miry · 2017-01-03T21:38:45Z

@e-dard who can merge this pr?

jwilder · 2017-01-04T21:20:13Z

Thanks @miry!

INFLUXDB_GRAPHITE_TEMPLATES expects multiple templates to be separated by ',' (see influxdata/influxdb#7323), in the same time ',' is used to separate multiple additional tags for a single template (see https://docs.influxdata.com/influxdb/v1.8/supported_protocols/graphite/#adding-tags) So unfortunately it's impossible to configure a template with multiple additional tags thru INFLUXDB_GRAPHITE_TEMPLATES env variable, and providing a config file with templates is the only available option for this. in order to check the whole influxdb config (including default values) you should start docker-compose setup and attach to the influxdb container (note that execution of docker compose commands is done in the metrics directory): cd metrics docker compose up -d docker compose exec influxdb bash then you can run this command in the influxdb container's shell: influxd config the default configuration file for influxdb container is located at '/etc/influxdb/influxdb.conf', you can print its content or check how the whole configuration looks like with it using these commands: cat /etc/influxdb/influxdb.conf INFLUXDB_CONFIG_PATH=/etc/influxdb/influxdb.conf influxd config

miry force-pushed the env-array-config branch from af25447 to a3b9e7c Compare September 19, 2016 08:31

gunnaraasen approved these changes Nov 30, 2016

View reviewed changes

miry force-pushed the env-array-config branch from 8cab286 to ae5478a Compare December 5, 2016 10:32

miry changed the title ~~Env array config~~ Allow add items to array config via ENV Dec 5, 2016

miry force-pushed the env-array-config branch from ae5478a to de2ad5b Compare December 5, 2016 10:35

miry force-pushed the env-array-config branch from de2ad5b to c2f5c2e Compare December 7, 2016 14:45

jwilder mentioned this pull request Dec 15, 2016

Cannot configure multiple graphite listeners via env variables? #7726

Closed

[influxdata#7323]: Allow add items to array config via ENV

65b08e5

Allow to create a new templates or tags configs, if there are no records in the default config. Fixes: influxdata#6943

miry force-pushed the env-array-config branch from c6e0f4e to 65b08e5 Compare December 23, 2016 08:21

e-dard approved these changes Dec 23, 2016

View reviewed changes

Merge branch 'master' into env-array-config

5ebd424

jwilder merged commit 1591544 into influxdata:master Jan 4, 2017

jwilder added this to the 1.2.0 milestone Jan 4, 2017

miry deleted the env-array-config branch January 5, 2017 10:38

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow add items to array config via ENV #7323

Allow add items to array config via ENV #7323

miry commented Sep 19, 2016 •

edited

Loading

mention-bot commented Sep 19, 2016

miry commented Nov 22, 2016

e-dard commented Nov 23, 2016

jackzampolin commented Nov 23, 2016

gunnaraasen left a comment

gunnaraasen Nov 29, 2016

miry Dec 1, 2016

gunnaraasen Nov 29, 2016

miry Dec 1, 2016

gunnaraasen Nov 30, 2016

miry Dec 1, 2016

gunnaraasen Nov 30, 2016

miry Dec 1, 2016 •

edited

Loading

miry commented Dec 1, 2016

e-dard commented Dec 1, 2016

e-dard commented Dec 1, 2016

miry commented Dec 1, 2016

miry commented Dec 5, 2016

fdrouet commented Dec 7, 2016

burdandrei commented Dec 22, 2016

miry commented Dec 23, 2016

miry commented Jan 3, 2017

jwilder commented Jan 4, 2017

Allow add items to array config via ENV #7323

Allow add items to array config via ENV #7323

Conversation

miry commented Sep 19, 2016 • edited Loading

Required for all non-trivial PRs

Required only if applicable

mention-bot commented Sep 19, 2016

miry commented Nov 22, 2016

e-dard commented Nov 23, 2016

jackzampolin commented Nov 23, 2016

gunnaraasen left a comment

Choose a reason for hiding this comment

gunnaraasen Nov 29, 2016

Choose a reason for hiding this comment

miry Dec 1, 2016

Choose a reason for hiding this comment

gunnaraasen Nov 29, 2016

Choose a reason for hiding this comment

miry Dec 1, 2016

Choose a reason for hiding this comment

gunnaraasen Nov 30, 2016

Choose a reason for hiding this comment

miry Dec 1, 2016

Choose a reason for hiding this comment

gunnaraasen Nov 30, 2016

Choose a reason for hiding this comment

miry Dec 1, 2016 • edited Loading

Choose a reason for hiding this comment

miry commented Dec 1, 2016

e-dard commented Dec 1, 2016

e-dard commented Dec 1, 2016

miry commented Dec 1, 2016

miry commented Dec 5, 2016

fdrouet commented Dec 7, 2016

burdandrei commented Dec 22, 2016

miry commented Dec 23, 2016

miry commented Jan 3, 2017

jwilder commented Jan 4, 2017

miry commented Sep 19, 2016 •

edited

Loading

miry Dec 1, 2016 •

edited

Loading