Skip to content

docs: drop unnecessary field in DogStatsdSink message#545

Merged
htuch merged 3 commits intoenvoyproxy:masterfrom
taiki45:fix-wrong-doc-in-dog-statsd-sink
Mar 15, 2018
Merged

docs: drop unnecessary field in DogStatsdSink message#545
htuch merged 3 commits intoenvoyproxy:masterfrom
taiki45:fix-wrong-doc-in-dog-statsd-sink

Conversation

@taiki45
Copy link
Copy Markdown
Member

@taiki45 taiki45 commented Mar 14, 2018

This is a follow-up PR of #325.
I mistook to add an unnecessary field here. What actually we need is just making dog_statsd_specifier oneof field, not adding tcp_cluster_name field.

We can safely drop this field because users has ended up with
initializing errors if they had specified this field.

[critical][main] source/server/server.cc:71] error initializing configuration '/envoy.yaml': Address must be a socket or pipe

This field has
never been used in envoy repo: https://github.com/envoyproxy/envoy/blob/7d03b231c7e8e086956096f6ca961f82c684ed0b/source/server/config/stats/dog_statsd.cc#L19-L23

Signed-off-by: Taiki Ono taiki-ono@cookpad.com

We can safely drop this field because users has ended up with
initializing errors if they had specified this field. This field has
never been used in envoy repo.

Signed-off-by: Taiki Ono <taiki-ono@cookpad.com>

// The name of a cluster that is DogStatsD compliant TCP listener. If specified,
// Envoy will connect to this cluster to flush statistics.
string tcp_cluster_name = 2;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks. Can you add a comment like

// [#comment:next free field: 30]
to this message, so that we skip to field number 3 next? Even if nobody is using this field, we don't want to reuse the field number for wire compatibility reasons.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Cool. Is this correct? e6fa3f7

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

You should also add a reserved field for number 2, as describe here: https://github.com/envoyproxy/data-plane-api/blame/master/STYLE.md#L26

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I missed the document, thanks. 36500bf

Taiki Ono added 2 commits March 15, 2018 03:38
Signed-off-by: Taiki Ono <taiki-ono@cookpad.com>
Signed-off-by: Taiki Ono <taiki-ono@cookpad.com>
@htuch htuch merged commit f1c7a98 into envoyproxy:master Mar 15, 2018
@taiki45 taiki45 deleted the fix-wrong-doc-in-dog-statsd-sink branch March 15, 2018 14:32
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