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

server: expose flushStats() method #10097

Merged
merged 10 commits into from
Feb 25, 2020
Merged

server: expose flushStats() method #10097

merged 10 commits into from
Feb 25, 2020

Conversation

junr03
Copy link
Member

@junr03 junr03 commented Feb 19, 2020

Description: this PR moves the previously private flushStats method in the server instance to the public interface. This method can be used by consumers to flush stats out-of-band from the stats flushing interval.

In particular it would help API driven consumers like Envoy Mobile to flush relevant stats on-demand. Related issue in Envoy Mobile envoyproxy/envoy-mobile#573
Risk Level: low
Testing: updated mocks. updated TEST_P(ServerStatsTest, FlushStats) to use the exposed function.

Signed-off-by: Jose Nino jnino@lyft.com

Jose Nino added 2 commits February 18, 2020 16:21
Signed-off-by: Jose Nino <jnino@lyft.com>
Signed-off-by: Jose Nino <jnino@lyft.com>
@junr03 junr03 changed the title Flushstats server: expose flushStats() method Feb 19, 2020
jmarantz
jmarantz previously approved these changes Feb 19, 2020
include/envoy/server/instance.h Outdated Show resolved Hide resolved
Signed-off-by: Jose Nino <jnino@lyft.com>
Signed-off-by: Jose Nino <jnino@lyft.com>
Signed-off-by: Jose Nino <jnino@lyft.com>
Jose Nino added 2 commits February 21, 2020 12:19
Signed-off-by: Jose Nino <jnino@lyft.com>
Signed-off-by: Jose Nino <jnino@lyft.com>
jmarantz
jmarantz previously approved these changes Feb 22, 2020
Copy link
Contributor

@jmarantz jmarantz left a comment

Choose a reason for hiding this comment

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

One one-word nit, otherwise lgtm

@envoyproxy/senior-maintainers for a final pass.

include/envoy/server/instance.h Outdated Show resolved Hide resolved
Signed-off-by: Jose Nino <jnino@lyft.com>
Signed-off-by: Jose Nino <jnino@lyft.com>
jmarantz
jmarantz previously approved these changes Feb 22, 2020
include/envoy/server/instance.h Outdated Show resolved Hide resolved
Signed-off-by: Jose Nino <jnino@lyft.com>
@jmarantz
Copy link
Contributor

/retest

@repokitteh-read-only
Copy link

🔨 rebuilding ci/circleci: coverage (failed build)

🐱

Caused by: a #10097 (comment) was created by @jmarantz.

see: more, trace.

@mattklein123 mattklein123 merged commit b6d1fec into envoyproxy:master Feb 25, 2020
rebello95 added a commit to envoyproxy/envoy-mobile that referenced this pull request Feb 27, 2020
Bumping to include envoyproxy/envoy#10097, which we'll be utilizing shortly.

Signed-off-by: Michael Rebello <me@michaelrebello.com>
rebello95 added a commit to envoyproxy/envoy-mobile that referenced this pull request Feb 27, 2020
Bumping to include envoyproxy/envoy#10097, which we'll be utilizing shortly.

Signed-off-by: Michael Rebello <me@michaelrebello.com>
rebello95 added a commit to envoyproxy/envoy-mobile that referenced this pull request Feb 27, 2020
Adds functionality built on top of the upstream Envoy changes in envoyproxy/envoy#10097 which will allow the bridging layers to manually flush stats based on lifecycle changes in the mobile application.

Follow-up PRs will call these functions from Java/Objective-C.

Part of #573.

Signed-off-by: Michael Rebello <me@michaelrebello.com>
rebello95 added a commit to envoyproxy/envoy-mobile that referenced this pull request Mar 3, 2020
Adds functionality built on top of the upstream Envoy changes in envoyproxy/envoy#10097 which will allow the bridging layers to manually flush stats based on lifecycle changes in the mobile application.

Note: Stats must be flushed from the main Envoy thread, as validated by the following assertion failure that occurs when attempting to flush from another thread:

```
[2020-02-27 14:38:43.181][4716580][critical][assert] [external/envoy/source/common/thread_local/thread_local_impl.cc:190] assert failure: std::this_thread::get_id() == main_thread_id_.
```

Follow-up PRs will call these functions from Java/Objective-C.

Part of #573.

Signed-off-by: Michael Rebello <me@michaelrebello.com>
rebello95 added a commit to envoyproxy/envoy-mobile that referenced this pull request Mar 3, 2020
Adds functionality built on top of the upstream Envoy changes in envoyproxy/envoy#10097 which will allow the bridging layers to manually flush stats based on lifecycle changes in the mobile application.

Note: Stats must be flushed from the main Envoy thread, as validated by the following assertion failure that occurs when attempting to flush from another thread:

```
[2020-02-27 14:38:43.181][4716580][critical][assert] [external/envoy/source/common/thread_local/thread_local_impl.cc:190] assert failure: std::this_thread::get_id() == main_thread_id_.
```

Follow-up PRs will call these functions from Java/Objective-C.

Part of #573.

Signed-off-by: Michael Rebello <me@michaelrebello.com>
rebello95 added a commit to envoyproxy/envoy-mobile that referenced this pull request Mar 3, 2020
Adds functionality built on top of the upstream Envoy changes in envoyproxy/envoy#10097 which will allow the bridging layers to manually flush stats based on lifecycle changes in the mobile application.

Note: Stats must be flushed from the main Envoy thread, as validated by the following assertion failure that occurs when attempting to flush from another thread:

```
[2020-02-27 14:38:43.181][4716580][critical][assert] [external/envoy/source/common/thread_local/thread_local_impl.cc:190] assert failure: std::this_thread::get_id() == main_thread_id_.
```

Follow-up PRs will call these functions from Java/Objective-C.

Part of #573.

Signed-off-by: Michael Rebello <me@michaelrebello.com>
jpsim pushed a commit that referenced this pull request Nov 28, 2022
Bumping to include #10097, which we'll be utilizing shortly.

Signed-off-by: Michael Rebello <me@michaelrebello.com>
Signed-off-by: JP Simard <jp@jpsim.com>
jpsim pushed a commit that referenced this pull request Nov 28, 2022
Adds functionality built on top of the upstream Envoy changes in #10097 which will allow the bridging layers to manually flush stats based on lifecycle changes in the mobile application.

Note: Stats must be flushed from the main Envoy thread, as validated by the following assertion failure that occurs when attempting to flush from another thread:

```
[2020-02-27 14:38:43.181][4716580][critical][assert] [external/envoy/source/common/thread_local/thread_local_impl.cc:190] assert failure: std::this_thread::get_id() == main_thread_id_.
```

Follow-up PRs will call these functions from Java/Objective-C.

Part of envoyproxy/envoy-mobile#573.

Signed-off-by: Michael Rebello <me@michaelrebello.com>
Signed-off-by: JP Simard <jp@jpsim.com>
jpsim pushed a commit that referenced this pull request Nov 29, 2022
Bumping to include #10097, which we'll be utilizing shortly.

Signed-off-by: Michael Rebello <me@michaelrebello.com>
Signed-off-by: JP Simard <jp@jpsim.com>
jpsim pushed a commit that referenced this pull request Nov 29, 2022
Adds functionality built on top of the upstream Envoy changes in #10097 which will allow the bridging layers to manually flush stats based on lifecycle changes in the mobile application.

Note: Stats must be flushed from the main Envoy thread, as validated by the following assertion failure that occurs when attempting to flush from another thread:

```
[2020-02-27 14:38:43.181][4716580][critical][assert] [external/envoy/source/common/thread_local/thread_local_impl.cc:190] assert failure: std::this_thread::get_id() == main_thread_id_.
```

Follow-up PRs will call these functions from Java/Objective-C.

Part of envoyproxy/envoy-mobile#573.

Signed-off-by: Michael Rebello <me@michaelrebello.com>
Signed-off-by: JP Simard <jp@jpsim.com>
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