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.

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

Document complex object support in env vars #4200

Merged
merged 3 commits into from
Jun 9, 2017

Conversation

dedemorton
Copy link
Contributor

Adds info requested in issue #3686 . Also did some reorganizing so that we no longer have duplicated content in two places. Had to add a little conditional coding to embed the file.

Please respond to my questions inline.

@dedemorton dedemorton changed the title Document complex ojbect support in env vars Document complex object support in env vars May 4, 2017
You can specify complex objects, such as lists or dictionaries, in environment
variables by using a superset of JSON.

//REVIEWERS: Not sure I like describing this as a "superset of JSON." Would it make sense to say "by using a JSON-like syntax"?
Copy link
Contributor

Choose a reason for hiding this comment

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

+1 for "JSON-like syntax"

-------------------------------------------------------------------------------


//REVIEWERS: Does anyone have a better example that is less Filebeat-specific?
Copy link
Contributor

Choose a reason for hiding this comment

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

The typical example I've seen is adding multiple Elasticsearch hosts.

@@ -32,8 +39,19 @@ undefined.
If you need to use a literal `${` in your configuration file then you can write
`$${` to escape the expansion.

// REVIEWERS: Is the following statement still true? "The `$` symbol can be used to escape othercharacters in the default_value like using `$}` in order to generate a `}` character without closing the variable expansion."
Copy link

Choose a reason for hiding this comment

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

Yes, still should be true.

@@ -49,3 +67,36 @@ and what each configuration looks like after replacement:
|`name: ${NAME:beats}` |no setting |`name: beats`
|`name: ${NAME:beats}` |`export NAME=elastic` |`name: elastic`
|==================================

[float]
=== Specifying Complex Objects in Environment Variables
Copy link

Choose a reason for hiding this comment

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

Note: the same syntax for complex objects can be used via -E setting as well.

e.g. I can do from command line: -E output='{elasticsearch.enabled: false, console.pretty: true}', to disable the ES output and have all events written to console.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will add to the topic that discusses the -E option

When you specify complex objects in environment variables, keep the following
in mind:

* Strings can be unquoted, single-quoted, or double-quoted
Copy link

Choose a reason for hiding this comment

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

Note: the quotation rules are for a) convenience in simple settings b) different quotation rules for somewhat safely mixing quotation in shell itself.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added clarification.


* Strings can be unquoted, single-quoted, or double-quoted
* Arrays at the top-level do not require brackets (`[]`). Instead, use commas (`,`) to
separate the elements.
Copy link

Choose a reason for hiding this comment

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

Sounds to me like we discourage (don't allow/want) users to use [] for top-level arrays. Especially for more complex objects I'd encourage [] even on top-level. It's just [] is optional for top-level arrays for user convience only.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

@dedemorton dedemorton force-pushed the beats_issue#3686 branch 2 times, most recently from 9a68e1b to 82edcaf Compare May 17, 2017 19:50
@dedemorton
Copy link
Contributor Author

@urso Added the changes you suggested. I'm not sure if I got the wording quite right on the bit about quotations (let me know). Let me know if you think it's not clear enough. I think it's probably good enough just to explain that the flexibility is there...I don't want to go into too much detail.

=== Specifying Complex Objects in Environment Variables

You can specify complex objects, such as lists or dictionaries, in environment
variables by using a JSON-like syntax.
Copy link

Choose a reason for hiding this comment

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

maybe we want to be more explicit here, as JSON-like syntax is pretty unspecific on syntax limitations + move the pointers from line 93pp up here.

e.g.:

Like with JSON, dictionaries and lists are constructed using {} and []. But in comparison to JSON, the syntax allows for trailing commas and slightly different string quotation rules. Strings can be unquoted, single-quoted, or double-quoted, as a convenience for simple settings and to make it easier for you to mix quotation usage in the shell. Arrays at the top-level do not require brackets ([]).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good idea. fixed.

["source","sh"]
--------------------------------------------------------------------------------
-E output='{elasticsearch.enabled: false, console.pretty: true}'
--------------------------------------------------------------------------------
Copy link

Choose a reason for hiding this comment

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

maybe include sample config and final config here?

e.g. with -E output='{elasticsearch.enabled: false, console.pretty: true}'

and a config file like:

output.elasticsearch:
  hosts: ["http://localhost:9200"]
  username: username
  password: password

then the final configuration presented to the beat becomes:

output.elasticsearch:
  enabled: false
  hosts: ["http://localhost:9200"]
  username: username
  password: password

output.console:
  pretty: true

That is, overwriting fields with complex objects will merge the object with the original configuration, not changing/overwriting any other settings.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added

@@ -49,3 +65,38 @@ and what each configuration looks like after replacement:
|`name: ${NAME:beats}` |no setting |`name: beats`
|`name: ${NAME:beats}` |`export NAME=elastic` |`name: elastic`
|==================================
Copy link

Choose a reason for hiding this comment

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

regarding this table, 2 syntax types are missing:

According to this we've got 3 kind of operators:

: is default value operator
:? is the error message operator. If the environment variable can not be expanded, the error message will contain the custom error message... e.g. name: ${NAME:?You need to set the NAME environment variable} will have the error message contain the string You need to set the NAME environment variable, if NAME is not set.
:+ is alternate value operator (if variable is set, expand with alternative value)
e.g. name: ${NAME:+beats} with export NAME=elastic becomes name: beats. By nesting variable expansions with default cases and alternative rules one can build some kind of negation and checks. This is quite some advanced use-case almost no one will use in practice (it's somewhat tricky to use this one right). e.g. use case include other string if variable is set:

cmd: a b ${param:+-x ${param}}

will expand to cmd: a b -x y if export param=y and cmd: a b only if param is not set.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@urso I'm looking at this in 6.0.0-alpha2, and the error message with the custom text isn't what you would expect. When name: ${NAME:?You need to set the NAME environment variable} is set in the config and the NAME env var can't be resolved, the Beat returns the error You need to set the NAME environment variable accessing 'name' (source:'filebeat.yml'). (Whatever you specify for custom text gets prepended to accessing 'name' (source:'filebeat.yml'). I'll push the changes I made, but I'm not sure we should document the error syntax since it results in weird messages.

Also, since the alternate value operator is an advanced use case that almost no one will use in practice AND it's tricky to use, I don't think we should document it. We might cause more problems than we solve by documenting it. WDYT?

Copy link

@urso urso Jun 9, 2017

Choose a reason for hiding this comment

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

oh, I did forget that go-ucfg is always appending the full settings name and source when reporting an error.

I'm ok with not documenting the :+ operator, as it's somewhat complex. About the error operator :? I'm somewhat on the fence. I'm ok with just leaving it out or maybe adjusting the error message in go-ucfg itself.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@urso I think we should document the :? operator and adjust the go-ucfg message. It seems likely that some users will want to return an error that provides some context when a variable cannot be resolved.

@urso
Copy link

urso commented Jun 9, 2017

LGTM

@dedemorton dedemorton merged commit 991b6cc into elastic:master Jun 9, 2017
@dedemorton dedemorton added the needs_backport PR is waiting to be backported to other branches. label Jun 9, 2017
dedemorton added a commit to dedemorton/beats that referenced this pull request Jun 16, 2017
* Document complex ojbect support in env vars

* Fixes from the review

* More changes from the review
dedemorton added a commit to dedemorton/beats that referenced this pull request Jun 16, 2017
* Document complex ojbect support in env vars

* Fixes from the review

* More changes from the review
@dedemorton dedemorton removed the needs_backport PR is waiting to be backported to other branches. label Jun 16, 2017
andrewkroh pushed a commit that referenced this pull request Jun 16, 2017
* Document complex ojbect support in env vars

* Fixes from the review

* More changes from the review
dedemorton added a commit that referenced this pull request Jun 21, 2017
leweafan pushed a commit to leweafan/beats that referenced this pull request Apr 28, 2023
* Document complex ojbect support in env vars

* Fixes from the review

* More changes from the review
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants