Skip to content

Conversation

@mkocher
Copy link
Member

@mkocher mkocher commented Sep 16, 2023

  • In general Diego does not generate container metrics for tasks. There was no way to disable log rate metrics for tasks without this change.
  • Add a bool to the AllocationRequest, save that off, and when the container is run set the metricReportInterval to zero if GenerateLogMetrics is false for that container.
  • Please note that this change will need to be made in lockstep with the Rep change as the structure of the AllocationRequest has changed.

What is this change about?

Container log rate limit metrics

What problem it is trying to solve?

Tasks never had container metrics, and when we introduced log rate limit metrics we inadvertently introduced them to all containers, not just LRP containers.

What is the impact if the change is not made?

Task containers will continue to generate log rate metrics (that don't have the correct tags) and no other container metrics

How should this change be described in diego-release release notes?

Log rate limit metrics are no longer generated for tasks

Please provide any contextual information.

Tag your pair, your PM, and/or team!

@rroberts2222

- In general Diego does not generate container metrics for tasks. There
  was no way to disable log rate metrics for tasks without this change.
- Add a bool to the AllocationRequest, save that off, and when the
  container is run set the metricReportInterval to zero if
  GenerateLogMetrics is false for that container.
- Please note that this change will need to be made in lockstep with the
  Rep change as the structure of the AllocationRequest has changed.

Signed-off-by: Matthew Kocher <[email protected]>
@MarcPaquette MarcPaquette merged commit d3b3018 into cloudfoundry:main Oct 12, 2023
jrussett added a commit to cloudfoundry/inigo that referenced this pull request Oct 19, 2023
Fixes:
```
vet: inigo/executor/executor_garden_test.go:153:95: not enough arguments in call to executor.NewAllocationRequest
	have (string, *executor.Resource, executor.Tags)
	want (string, *executor.Resource, bool, executor.Tags)
vet: inigo/volman/volman_executor_test.go:193:87: not enough arguments in call to executor.NewAllocationRequest
	have (string, *executor.Resource, executor.Tags)
	want (string, *executor.Resource, bool, executor.Tags)
```

Also see this PR/commit for why it changed:
- cloudfoundry/executor#86
- cloudfoundry/executor@d3b3018
jrussett added a commit to cloudfoundry/inigo that referenced this pull request Oct 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants