-
Notifications
You must be signed in to change notification settings - Fork 3
fix: debug event timezone handling #79
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
Merged
Merged
Changes from all commits
Commits
Show all changes
26 commits
Select commit
Hold shift + click to select a range
7984adc
add beginnings of sdk contract test server
cwaldren-ld 725e05e
trying to make client movable
cwaldren-ld c7218ed
fix non-movable Session object
cwaldren-ld 565c8ea
fixing contract tests
cwaldren-ld 6d66f9e
allow for null evaluation reason when deserializing flag evaluation r…
cwaldren-ld d7dca32
add github action for SDK tests
cwaldren-ld 85ece7b
unify the github actions for sse/sdk contrac tests
cwaldren-ld 4a754a0
fix log tag on sdk-contract-test server
cwaldren-ld 1df9646
feat: implement AllFlags [contract-tests] (#53)
cwaldren-ld e3f3e85
pass all context type/build tests
cwaldren-ld 97cfe51
pass more tests
cwaldren-ld bb3c47e
update suppressions
cwaldren-ld fb685d4
fix: send identify event on client creation (#54)
cwaldren-ld 717a968
Merge branch 'cw/sc-195303/contract-tests' into cw/parse-contexts
cwaldren-ld c7bbe7f
have ContextFilter take the global private attributes by value
cwaldren-ld 753b138
Merge branch 'cw/sc-195303/contract-tests' into cw/parse-contexts
cwaldren-ld 1cdbbf8
make context parsing slightly cleaner
cwaldren-ld 462f32a
add json error ToString function
cwaldren-ld f83c44a
updating date parsing logic to account for local time offset
cwaldren-ld dc12b80
add invalid date test
cwaldren-ld e1ba1c0
remove debug event tests from suppressions
cwaldren-ld 1a7e6b8
Merge branch 'main' into cw/debug-event-fixes
cwaldren-ld 85979a0
remove obsolete code
cwaldren-ld 07edba9
make ParseValidDateHeader test not dependent on time unit count
cwaldren-ld 165899e
remove unecessary headers
cwaldren-ld 3794989
lints
cwaldren-ld File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,9 +5,6 @@ | |
| #include <boost/lexical_cast.hpp> | ||
| #include <boost/uuid/uuid_io.hpp> | ||
| #include "config/detail/builders/http_properties_builder.hpp" | ||
| #include "config/detail/sdks.hpp" | ||
| #include "network/detail/asio_requester.hpp" | ||
|
|
||
| #include "serialization/events/json_events.hpp" | ||
|
|
||
| namespace http = boost::beast::http; | ||
|
|
@@ -111,7 +108,7 @@ void AsioEventProcessor<SDK>::HandleSend(InputEvent event) { | |
| template <typename SDK> | ||
| void AsioEventProcessor<SDK>::Flush(FlushTrigger flush_type) { | ||
| workers_.Get([this](RequestWorker* worker) { | ||
| if (!worker) { | ||
| if (worker == nullptr) { | ||
| LD_LOG(logger_, LogLevel::kDebug) | ||
| << "event-processor: no flush workers available; skipping " | ||
| "flush"; | ||
|
|
@@ -143,8 +140,8 @@ void AsioEventProcessor<SDK>::OnEventDeliveryResult( | |
| boost::ignore_unused(event_count); | ||
|
|
||
| std::visit( | ||
| overloaded{[&](Clock::time_point&& server_time) { | ||
| last_known_past_time_ = std::move(server_time); | ||
| overloaded{[&](Clock::time_point server_time) { | ||
| last_known_past_time_ = server_time; | ||
| }, | ||
| [&](network::detail::HttpResult::StatusCode status) { | ||
| std::lock_guard<std::mutex> guard{this->inbox_mutex_}; | ||
|
|
@@ -153,7 +150,7 @@ void AsioEventProcessor<SDK>::OnEventDeliveryResult( | |
| permanent_delivery_failure_ = true; | ||
| } | ||
| }}, | ||
| std::move(result)); | ||
| result); | ||
| } | ||
|
|
||
| template <typename SDK> | ||
|
|
@@ -220,29 +217,33 @@ std::vector<OutputEvent> AsioEventProcessor<SDK>::Process( | |
| overloaded{[&](client::FeatureEventParams&& event) { | ||
| summarizer_.Update(event); | ||
|
|
||
| if (!event.require_full_event) { | ||
| return; | ||
| } | ||
|
|
||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. bugfix: debug event emit logic shouldn't be gated by |
||
| client::FeatureEventBase base{event}; | ||
|
|
||
| auto debug_until_date = event.debug_events_until_date; | ||
|
|
||
| auto max_time = std::max( | ||
| // To be conservative, use as the current time the | ||
| // maximum of the actual current time and the server's | ||
| // time. This way if the local host is running behind, we | ||
| // won't accidentally keep emitting events. | ||
|
|
||
| auto conservative_now = std::max( | ||
| std::chrono::system_clock::now(), | ||
| last_known_past_time_.value_or( | ||
| std::chrono::system_clock::time_point{})); | ||
| std::chrono::system_clock::from_time_t(0))); | ||
|
|
||
| bool emit_debug_event = | ||
| debug_until_date && debug_until_date->t > max_time; | ||
| debug_until_date && | ||
| conservative_now < debug_until_date->t; | ||
|
|
||
| if (emit_debug_event) { | ||
| out.emplace_back(client::DebugEvent{ | ||
| base, filter_.filter(event.context)}); | ||
| } | ||
|
|
||
| out.emplace_back(client::FeatureEvent{ | ||
| std::move(base), event.context.kinds_to_keys()}); | ||
| if (event.require_full_event) { | ||
| out.emplace_back(client::FeatureEvent{ | ||
| std::move(base), event.context.kinds_to_keys()}); | ||
| } | ||
| }, | ||
| [&](client::IdentifyEventParams&& event) { | ||
| // Contexts should already have been checked for | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bugfix: simply parsing using
std::get_timefollowed bymktimegives us astd::chrono::system_clock::time_pointin local time. We need to actually compute the offset and subtract it out.This code is just so painful. C++20 has a
parsemethod which might help, or we can import a 3rd party library..