cloudfoundry-community / stackdriver-tools

Stackdriver Nozzle for Cloud Foundry Loggregator, Host Monitoring Agents BOSH Release
Apache License 2.0
21 stars 13 forks source link

Make nozzle buffer size configurable via manifest #138

Closed knyar closed 6 years ago

knyar commented 6 years ago

Also, rename batch_count and batch_duration internally to make it more obvious that they only apply to Stackdriver logging.


This change is Reviewable

johnsonj commented 6 years ago

Review status: 0 of 4 files reviewed at latest revision, 2 unresolved discussions.


jobs/stackdriver-nozzle/spec, line 39 at r1 (raw file):

    description: Contents of application_default_credentials.json, see https://cloud.google.com/logging/docs/agent/authorization#configuring_client_id_authorization.

  nozzle.metrics_buffer_duration:

Can you add these fields in tile.yml.erb as well? Use the same name, description, default values as here for the forms section

Pipe the values through here to the bosh manifest


src/stackdriver-nozzle/config/config.go, line 61 at r1 (raw file):

  // Stackdriver config
  ProjectID            string `envconfig:"gcp_project_id"`
  LoggingBatchCount    int    `envconfig:"batch_count" default:"10"`

update envconfig annotations here and for batch_duration


Comments from Reviewable

knyar commented 6 years ago

Review status: all files reviewed at latest revision, 2 unresolved discussions.


src/stackdriver-nozzle/config/config.go, line 61 at r1 (raw file):

Previously, johnsonj (Jeff Johnson) wrote…
update envconfig annotations here and for `batch_duration`

Any concerns about breaking existing users who might be setting variables with current names?


Comments from Reviewable

knyar commented 6 years ago

Review status: 2 of 6 files reviewed at latest revision, 2 unresolved discussions.


jobs/stackdriver-nozzle/spec, line 39 at r1 (raw file):

Previously, johnsonj (Jeff Johnson) wrote…
Can you add these fields in [tile.yml.erb](https://github.com/cloudfoundry-community/stackdriver-tools/blob/develop/tile.yml.erb#L41) as well? Use the same name, description, default values as here for the forms section Pipe the values through [here](https://github.com/cloudfoundry-community/stackdriver-tools/blob/develop/tile.yml.erb#L26) to the bosh manifest

Done.


src/stackdriver-nozzle/config/config.go, line 61 at r1 (raw file):

Previously, knyar (Anton Tolchanov) wrote…
Any concerns about breaking existing users who might be setting variables with current names?

I guess no concerns since these are not configurable via tile properties or manifest. Renamed, updated README.md.


Comments from Reviewable

johnsonj commented 6 years ago

Review status: 2 of 6 files reviewed at latest revision, 2 unresolved discussions.


src/stackdriver-nozzle/config/config.go, line 61 at r1 (raw file):

Previously, knyar (Anton Tolchanov) wrote…
I guess no concerns since these are not configurable via tile properties or manifest. Renamed, updated README.md.

Thanks! The only supported ways to use the nozzle are via bosh release/tile. If we do change a field there we will need to cover it in the release notes.


Comments from Reviewable

johnsonj commented 6 years ago

LGTM


Reviewed 4 of 4 files at r2. Review status: all files reviewed at latest revision, 2 unresolved discussions.


Comments from Reviewable