agent icon indicating copy to clipboard operation
agent copied to clipboard

Services cannot have non-optional fields

Open tpaschalis opened this issue 1 year ago • 1 comments

What's wrong?

When experimenting with a service that ~should~ could have a non-optional field, and when adding a module in the mix, I realized that the service block was also being evaluated within the module.

That means that services cannot have non-optional fields as they cannot be defined inside of modules.

Steps to reproduce

  • Add a required field to a service (eg. https://github.com/tpaschalis/agent/commit/4935c383d0d9fae81f1303f3649aa6421bb718bd)
  • Build the agent and run with a configuration file that contains a module
module.string "local_foo" {
	content = `prometheus.exporter.agent "own" {}`
}
  • Run the Agent
./build/grafana-agent-flow run ~/river-configs/remote-configuration.river
ts=2024-02-01T13:24:26.856309Z level=info "boringcrypto enabled"=false
ts=2024-02-01T13:24:26.861481Z level=info msg="starting complete graph evaluation" controller_id="" trace_id=f407e8a17f6a97421c05bfc2819f2086
...
...
Error: -: Failed to evaluate service: decoding River: missing required attribute "id"

Error: -: Failed to evaluate service: decoding River: missing required attribute "id"

interrupt received
  • Notice that the error appears twice and try to add the required argument. Since you cannot have service blocks defined in modules, so there's no way to fix this as a user.

System information

macOS

Software version

main (73253f7a5bba658a7111e37ae70a32a23a35de18)

Configuration

No response

Logs

No response

tpaschalis avatar Feb 01 '24 13:02 tpaschalis

If a service has non-optional arguments, it must not run by default. Else ALL config would need to contain the service block. So it should not be possible to end up in the situation described above.

If we want to allow optional arguments for services, then we need to allow services to be instantiated only if there is a corresponding block in the config (like components). If we allow this, then we woul have the duplicated error message for every module if a non-optional arg is missing.

wildum avatar Feb 05 '24 14:02 wildum

This issue has not had any activity in the past 30 days, so the needs-attention label has been added to it. If the opened issue is a bug, check to see if a newer release fixed your issue. If it is no longer relevant, please feel free to close this issue. The needs-attention label signals to maintainers that something has fallen through the cracks. No action is needed by you; your issue will be kept open and you do not have to respond to this comment. The label will be removed the next time this job runs if there is new activity. Thank you for your contributions!

github-actions[bot] avatar Mar 07 '24 00:03 github-actions[bot]

I'm closing this one out for now, as it's not exactly a bug and I don't see an urgent need for a non-runnable-by-default service.

tpaschalis avatar Mar 07 '24 16:03 tpaschalis