You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@trafficserver.apache.org by fpesce <gi...@git.apache.org> on 2015/06/17 23:45:37 UTC
[GitHub] trafficserver pull request: TS-2150: Add Milestone log tags
GitHub user fpesce opened a pull request:
https://github.com/apache/trafficserver/pull/229
TS-2150: Add Milestone log tags
- Add Milestone log tags
- Add Milestone diff log tags
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/fpesce/trafficserver TS-2150
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/trafficserver/pull/229.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #229
----
commit 93abcbcc62c98ac23a230fd82d33bf5ee1f01398
Author: Francois Pesce <fp...@yahoo-inc.com>
Date: 2015-06-17T05:38:27Z
TS-2150: Transaction milestone log tag
It includes a refactoring for Milestone class attributes to be
requested using public accessor on a private array indexed with an
enum.
Also a minor feature fix, which is aggregate log tag detection, that
matched aggregate string (like SUM FIRST etc..) using strstr() inside
the log tag preventing use of these string to be used in container
field.
commit e76fcc56b524b8b3bc837c02d08abe629c21389a
Author: Francois Pesce <fp...@yahoo-inc.com>
Date: 2015-06-17T21:42:40Z
TS-2150 documentation update
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request: TS-2150: Add Milestone log tags
Posted by SolidWallOfCode <gi...@git.apache.org>.
Github user SolidWallOfCode commented on the pull request:
https://github.com/apache/trafficserver/pull/229#issuecomment-116773852
I discussed this with Bryan and Susan and we think the least worst option is to put two milestone indices in the `LogField` class. The code in `LogFormat.cc` that creates the `LogField` instances has no provisions for creating sublcasses, which IMHO is a problem but outside the scope of this bug. Therefore we recommend adding two milestone indices to `LogField` and storing the appropriate milestone indices there in the constructor (by doing the lookup in the map) and then using those (instead of doing the lookup) during actual logging operations.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request: TS-2150: Add Milestone log tags
Posted by bryancall <gi...@git.apache.org>.
Github user bryancall commented on the pull request:
https://github.com/apache/trafficserver/pull/229#issuecomment-115746373
I think it would be better at the creation of the LogField store the index into the milestone array instead of looking it up every time you log. You can create a MilestoneLogFiled and have LogField as the base class if you don't want add a index to every LogField.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request: TS-2150: Add Milestone log tags
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/trafficserver/pull/229
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request: TS-2150: Add Milestone log tags
Posted by SolidWallOfCode <gi...@git.apache.org>.
Github user SolidWallOfCode commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/229#discussion_r33297453
--- Diff: proxy/logging/LogField.cc ---
@@ -132,6 +168,13 @@ LogField::LogField(const char *name, const char *symbol, Type type, MarshalFunc
m_time_field = (strcmp(m_symbol, "cqts") == 0 || strcmp(m_symbol, "cqth") == 0 || strcmp(m_symbol, "cqtq") == 0 ||
strcmp(m_symbol, "cqtn") == 0 || strcmp(m_symbol, "cqtd") == 0 || strcmp(m_symbol, "cqtt") == 0);
+
+ if (m_milestone_map.empty()) {
--- End diff --
This should really be in a local function or method, since it's used multiple times.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request: TS-2150: Add Milestone log tags
Posted by zwoop <gi...@git.apache.org>.
Github user zwoop commented on the pull request:
https://github.com/apache/trafficserver/pull/229#issuecomment-112961947
Please check with John (the current assignee of the TS-2150 Jira).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request: TS-2150: Add Milestone log tags
Posted by SolidWallOfCode <gi...@git.apache.org>.
Github user SolidWallOfCode commented on the pull request:
https://github.com/apache/trafficserver/pull/229#issuecomment-115379759
Looking much better. I think the `std::map` use is acceptable - it's not modified on the critical path and so there are no allocations which should be fine. Searching in `std::map` should be fast, especially for such a small set of strings. The initialization, though, shouldn't be cut and pasted so many places. Maybe that should be done in `Log::init_fields` as it is part of the field initialization.
My only other general comment is that sometimes `Milestone::diff` is used and sometimes the value are directly subtracted. That's a minor quibble, though. The real improvement there was from the millisecond computations which look a lot cleaner now.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request: TS-2150: Add Milestone log tags
Posted by fpesce <gi...@git.apache.org>.
Github user fpesce commented on the pull request:
https://github.com/apache/trafficserver/pull/229#issuecomment-115040061
I've removed the allocation, std::map is used only once (as a singleton to turn string from conf into enum value). Is there any lookup table system/class already existing that you would me to use instead?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request: TS-2150: Add Milestone log tags
Posted by SolidWallOfCode <gi...@git.apache.org>.
Github user SolidWallOfCode commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/229#discussion_r33060076
--- Diff: proxy/StatSystem.h ---
@@ -49,69 +49,75 @@
class TransactionMilestones
{
public:
+ // Used for array indexes, insert new value before _LAST one
+ enum Milestone {
+ ////////////////////////////////////////////////////////
+ // user agent: //
+ // user_agent_begin represents the time this //
+ // transaction started. If this is the first //
+ // transaction in a connection, then user_agent_begin //
+ // is set to accept time. otherwise it is set to //
+ // first read time. //
+ ////////////////////////////////////////////////////////
+ UA_BEGIN = 0,
+ UA_FIRST_READ,
+ UA_READ_HEADER_DONE,
+ UA_BEGIN_WRITE,
+ UA_CLOSE,
+
+ ////////////////////////////////////////////////////////
+ // server (origin_server , parent, blind tunnnel //
+ ////////////////////////////////////////////////////////
+ SERVER_FIRST_CONNECT,
+ SERVER_CONNECT,
+ SERVER_CONNECT_END,
+ SERVER_BEGIN_WRITE, // http only
+ SERVER_FIRST_READ, // http only
+ SERVER_READ_HEADER_DONE, // http only
+ SERVER_CLOSE,
+
+ CACHE_OPEN_READ_BEGIN,
+ CACHE_OPEN_READ_END,
+ CACHE_OPEN_WRITE_BEGIN,
+ CACHE_OPEN_WRITE_END,
+
+ DNS_LOOKUP_BEGIN,
+ DNS_LOOKUP_END,
+
+ ///////////////////
+ // state machine //
+ ///////////////////
+ SM_START,
+ SM_FINISH,
+
+ //////////////////
+ // API activity //
+ //////////////////
+ // Total amount of time spent in API calls converted to a timestamp.
+ // (subtract @a sm_start to get the actual time)
+ /// Time spent in API callout.
+ PLUGIN_ACTIVE,
+ /// Time spent in API state during the transaction.
+ PLUGIN_TOTAL,
+ LAST_ENTRY
+ // TODO: Should we instrument these at some point?
+ // CACHE_READ_BEGIN;
+ // CACHE_READ_END;
+ // CACHE_WRITE_BEGIN;
+ // CACHE_WRITE_END;
+ };
+
TransactionMilestones()
- : ua_begin(0), ua_first_read(0), ua_read_header_done(0), ua_begin_write(0), ua_close(0), server_first_connect(0),
- server_connect(0), server_connect_end(0), server_begin_write(0), server_first_read(0), server_read_header_done(0),
- server_close(0), cache_open_read_begin(0), cache_open_read_end(0), cache_open_write_begin(0), cache_open_write_end(0),
- dns_lookup_begin(0), dns_lookup_end(0), sm_start(0), sm_finish(0), plugin_active(0), plugin_total(0)
- {
- }
-
-
- ////////////////////////////////////////////////////////
- // user agent: //
- // user_agent_begin represents the time this //
- // transaction started. If this is the first //
- // transaction in a connection, then user_agent_begin //
- // is set to accept time. otherwise it is set to //
- // first read time. //
- ////////////////////////////////////////////////////////
- ink_hrtime ua_begin;
- ink_hrtime ua_first_read;
- ink_hrtime ua_read_header_done;
- ink_hrtime ua_begin_write;
- ink_hrtime ua_close;
-
- ////////////////////////////////////////////////////////
- // server (origin_server , parent, blind tunnnel //
- ////////////////////////////////////////////////////////
- ink_hrtime server_first_connect;
- ink_hrtime server_connect;
- ink_hrtime server_connect_end;
- ink_hrtime server_begin_write; // http only
- ink_hrtime server_first_read; // http only
- ink_hrtime server_read_header_done; // http only
- ink_hrtime server_close;
-
- ink_hrtime cache_open_read_begin;
- ink_hrtime cache_open_read_end;
- ink_hrtime cache_open_write_begin;
- ink_hrtime cache_open_write_end;
-
- ink_hrtime dns_lookup_begin;
- ink_hrtime dns_lookup_end;
-
- ///////////////////
- // state machine //
- ///////////////////
- ink_hrtime sm_start;
- ink_hrtime sm_finish;
-
- //////////////////
- // API activity //
- //////////////////
- // Total amount of time spent in API calls converted to a timestamp.
- // (subtract @a sm_start to get the actual time)
- /// Time spent in API callout.
- ink_hrtime plugin_active;
- /// Time spent in API state during the transaction.
- ink_hrtime plugin_total;
-
- // TODO: Should we instrument these at some point?
- // ink_hrtime cache_read_begin;
- // ink_hrtime cache_read_end;
- // ink_hrtime cache_write_begin;
- // ink_hrtime cache_write_end;
+ { // constructor to initialize array elements to zero in case user wants to
+ for (int i = 0; i < LAST_ENTRY; i++)
+ milestones[i] = 0;
+ }
--- End diff --
`ink_zero(milestones);`
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request: TS-2150: Add Milestone log tags
Posted by SolidWallOfCode <gi...@git.apache.org>.
Github user SolidWallOfCode commented on the pull request:
https://github.com/apache/trafficserver/pull/229#issuecomment-115738793
Ready to ship. The only tweak I'd recommend now is to put a reference to `TSHttpTxnMilestoneGet` in the documentation update so there's a link to the list of milestones.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request: TS-2150: Add Milestone log tags
Posted by SolidWallOfCode <gi...@git.apache.org>.
Github user SolidWallOfCode commented on the pull request:
https://github.com/apache/trafficserver/pull/229#issuecomment-114571019
I think the use of `std::map` is problematic because of its secondary effects, most importantly having to duplicate a string just to do a look up is a serious flaw IMHO. You might look at using `ts::Buffer` or `ts::ConstBuffer` as the key type instead. Then you can use a length based comparison functor for the `std::map` comparator.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request: TS-2150: Add Milestone log tags
Posted by bryancall <gi...@git.apache.org>.
Github user bryancall commented on the pull request:
https://github.com/apache/trafficserver/pull/229#issuecomment-114593823
We shouldn't be using std:map in the first place. It would duplicate the string object, but not the string bytes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request: TS-2150: Add Milestone log tags
Posted by SolidWallOfCode <gi...@git.apache.org>.
Github user SolidWallOfCode commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/229#discussion_r33062665
--- Diff: proxy/logging/LogField.cc ---
@@ -212,6 +269,51 @@ LogField::~LogField()
ats_free(m_symbol);
}
+TransactionMilestones::Milestone
+LogField::milestone_from_m_name(const char *m_name)
+{
+ milestone_map::iterator it;
+ TransactionMilestones::Milestone result = TransactionMilestones::LAST_ENTRY;
+
+ it = m_milestone_map.find(m_name);
+ if (it != m_milestone_map.end())
+ result = it->second;
+
+ return result;
+}
+
+int
+LogField::milestones_from_m_name(const char *m_name, TransactionMilestones::Milestone *ms1, TransactionMilestones::Milestone *ms2)
+{
+ milestone_map::iterator it;
+ char *ms_name;
+ size_t ms_name_len, skip;
+
+ ms_name_len = strcspn(m_name, " -");
+ ms_name = strndup(m_name, ms_name_len);
--- End diff --
It's very frowned upon to do allocations in the main path. The find method should be changed to accept a `char const*` and length and avoid the duplication / allocation here.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request: TS-2150: Add Milestone log tags
Posted by SolidWallOfCode <gi...@git.apache.org>.
Github user SolidWallOfCode commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/229#discussion_r33060489
--- Diff: proxy/http/HttpTransact.cc ---
@@ -8856,75 +8856,100 @@ HttpTransact::update_size_and_time_stats(State *s, ink_hrtime total_time, ink_hr
// update milestones stats
if (http_ua_begin_time_stat) {
- HTTP_SUM_TRANS_STAT(http_ua_begin_time_stat, milestone_difference_msec(milestones.sm_start, milestones.ua_begin))
+ HTTP_SUM_TRANS_STAT(http_ua_begin_time_stat, milestone_difference_msec(milestones.ms_get(TransactionMilestones::SM_START),
+ milestones.ms_get(TransactionMilestones::UA_BEGIN)))
--- End diff --
Here, at least, `milestone_difference_msec()` should take the enum values and do the `ms_get` internally. That would look a lot cleaner.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request: TS-2150: Add Milestone log tags
Posted by SolidWallOfCode <gi...@git.apache.org>.
Github user SolidWallOfCode commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/229#discussion_r33060322
--- Diff: proxy/http/HttpSM.cc ---
@@ -6662,48 +6668,61 @@ HttpSM::update_stats()
}
char client_ip[INET6_ADDRSTRLEN];
ats_ip_ntop(&t_state.client_info.addr, client_ip, sizeof(client_ip));
- Error("[%" PRId64 "] Slow Request: "
- "client_ip: %s:%u "
- "url: %s "
- "status: %d "
- "unique id: %s "
- "redirection_tries: %d "
- "bytes: %" PRId64 " "
- "fd: %d "
- "client state: %d "
- "server state: %d "
- "ua_begin: %.3f "
- "ua_first_read: %.3f "
- "ua_read_header_done: %.3f "
- "cache_open_read_begin: %.3f "
- "cache_open_read_end: %.3f "
- "dns_lookup_begin: %.3f "
- "dns_lookup_end: %.3f "
- "server_connect: %.3f "
- "server_first_read: %.3f "
- "server_read_header_done: %.3f "
- "server_close: %.3f "
- "ua_close: %.3f "
- "sm_finish: %.3f "
- "plugin_active: %.3f "
- "plugin_total: %.3f",
- sm_id, client_ip, ats_ip_port_host_order(&t_state.client_info.addr), url_string, status, unique_id_string,
- redirection_tries, client_response_body_bytes, fd, t_state.client_info.state, t_state.server_info.state,
- milestone_difference(milestones.sm_start, milestones.ua_begin),
- milestone_difference(milestones.sm_start, milestones.ua_first_read),
- milestone_difference(milestones.sm_start, milestones.ua_read_header_done),
- milestone_difference(milestones.sm_start, milestones.cache_open_read_begin),
- milestone_difference(milestones.sm_start, milestones.cache_open_read_end),
- milestone_difference(milestones.sm_start, milestones.dns_lookup_begin),
- milestone_difference(milestones.sm_start, milestones.dns_lookup_end),
- milestone_difference(milestones.sm_start, milestones.server_connect),
- milestone_difference(milestones.sm_start, milestones.server_first_read),
- milestone_difference(milestones.sm_start, milestones.server_read_header_done),
- milestone_difference(milestones.sm_start, milestones.server_close),
- milestone_difference(milestones.sm_start, milestones.ua_close),
- milestone_difference(milestones.sm_start, milestones.sm_finish),
- milestone_difference(milestones.sm_start, milestones.plugin_active),
- milestone_difference(milestones.sm_start, milestones.plugin_total));
+ Error(
+ "[%" PRId64 "] Slow Request: "
+ "client_ip: %s:%u "
+ "url: %s "
+ "status: %d "
+ "unique id: %s "
+ "redirection_tries: %d "
+ "bytes: %" PRId64 " "
+ "fd: %d "
+ "client state: %d "
+ "server state: %d "
+ "ua_begin: %.3f "
+ "ua_first_read: %.3f "
+ "ua_read_header_done: %.3f "
+ "cache_open_read_begin: %.3f "
+ "cache_open_read_end: %.3f "
+ "dns_lookup_begin: %.3f "
+ "dns_lookup_end: %.3f "
+ "server_connect: %.3f "
+ "server_first_read: %.3f "
+ "server_read_header_done: %.3f "
+ "server_close: %.3f "
+ "ua_close: %.3f "
+ "sm_finish: %.3f "
+ "plugin_active: %.3f "
+ "plugin_total: %.3f",
+ sm_id, client_ip, ats_ip_port_host_order(&t_state.client_info.addr), url_string, status, unique_id_string, redirection_tries,
+ client_response_body_bytes, fd, t_state.client_info.state, t_state.server_info.state,
+ milestone_difference(milestones.ms_get(TransactionMilestones::SM_START), milestones.ms_get(TransactionMilestones::UA_BEGIN)),
--- End diff --
Is this done enough to justify `TransactionMilestones::different(enum, enum)` ?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request: TS-2150: Add Milestone log tags
Posted by fpesce <gi...@git.apache.org>.
Github user fpesce commented on the pull request:
https://github.com/apache/trafficserver/pull/229#issuecomment-116774590
Looks like a good least worst idea to me. I'll update the PR.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request: TS-2150: Add Milestone log tags
Posted by SolidWallOfCode <gi...@git.apache.org>.
Github user SolidWallOfCode commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/229#discussion_r33060139
--- Diff: proxy/StatSystem.h ---
@@ -49,69 +49,75 @@
class TransactionMilestones
{
public:
+ // Used for array indexes, insert new value before _LAST one
+ enum Milestone {
+ ////////////////////////////////////////////////////////
+ // user agent: //
+ // user_agent_begin represents the time this //
+ // transaction started. If this is the first //
+ // transaction in a connection, then user_agent_begin //
+ // is set to accept time. otherwise it is set to //
+ // first read time. //
+ ////////////////////////////////////////////////////////
+ UA_BEGIN = 0,
+ UA_FIRST_READ,
+ UA_READ_HEADER_DONE,
+ UA_BEGIN_WRITE,
+ UA_CLOSE,
+
+ ////////////////////////////////////////////////////////
+ // server (origin_server , parent, blind tunnnel //
+ ////////////////////////////////////////////////////////
+ SERVER_FIRST_CONNECT,
+ SERVER_CONNECT,
+ SERVER_CONNECT_END,
+ SERVER_BEGIN_WRITE, // http only
+ SERVER_FIRST_READ, // http only
+ SERVER_READ_HEADER_DONE, // http only
+ SERVER_CLOSE,
+
+ CACHE_OPEN_READ_BEGIN,
+ CACHE_OPEN_READ_END,
+ CACHE_OPEN_WRITE_BEGIN,
+ CACHE_OPEN_WRITE_END,
+
+ DNS_LOOKUP_BEGIN,
+ DNS_LOOKUP_END,
+
+ ///////////////////
+ // state machine //
+ ///////////////////
+ SM_START,
+ SM_FINISH,
+
+ //////////////////
+ // API activity //
+ //////////////////
+ // Total amount of time spent in API calls converted to a timestamp.
+ // (subtract @a sm_start to get the actual time)
+ /// Time spent in API callout.
+ PLUGIN_ACTIVE,
+ /// Time spent in API state during the transaction.
+ PLUGIN_TOTAL,
+ LAST_ENTRY
+ // TODO: Should we instrument these at some point?
+ // CACHE_READ_BEGIN;
+ // CACHE_READ_END;
+ // CACHE_WRITE_BEGIN;
+ // CACHE_WRITE_END;
+ };
+
TransactionMilestones()
- : ua_begin(0), ua_first_read(0), ua_read_header_done(0), ua_begin_write(0), ua_close(0), server_first_connect(0),
- server_connect(0), server_connect_end(0), server_begin_write(0), server_first_read(0), server_read_header_done(0),
- server_close(0), cache_open_read_begin(0), cache_open_read_end(0), cache_open_write_begin(0), cache_open_write_end(0),
- dns_lookup_begin(0), dns_lookup_end(0), sm_start(0), sm_finish(0), plugin_active(0), plugin_total(0)
- {
- }
-
-
- ////////////////////////////////////////////////////////
- // user agent: //
- // user_agent_begin represents the time this //
- // transaction started. If this is the first //
- // transaction in a connection, then user_agent_begin //
- // is set to accept time. otherwise it is set to //
- // first read time. //
- ////////////////////////////////////////////////////////
- ink_hrtime ua_begin;
- ink_hrtime ua_first_read;
- ink_hrtime ua_read_header_done;
- ink_hrtime ua_begin_write;
- ink_hrtime ua_close;
-
- ////////////////////////////////////////////////////////
- // server (origin_server , parent, blind tunnnel //
- ////////////////////////////////////////////////////////
- ink_hrtime server_first_connect;
- ink_hrtime server_connect;
- ink_hrtime server_connect_end;
- ink_hrtime server_begin_write; // http only
- ink_hrtime server_first_read; // http only
- ink_hrtime server_read_header_done; // http only
- ink_hrtime server_close;
-
- ink_hrtime cache_open_read_begin;
- ink_hrtime cache_open_read_end;
- ink_hrtime cache_open_write_begin;
- ink_hrtime cache_open_write_end;
-
- ink_hrtime dns_lookup_begin;
- ink_hrtime dns_lookup_end;
-
- ///////////////////
- // state machine //
- ///////////////////
- ink_hrtime sm_start;
- ink_hrtime sm_finish;
-
- //////////////////
- // API activity //
- //////////////////
- // Total amount of time spent in API calls converted to a timestamp.
- // (subtract @a sm_start to get the actual time)
- /// Time spent in API callout.
- ink_hrtime plugin_active;
- /// Time spent in API state during the transaction.
- ink_hrtime plugin_total;
-
- // TODO: Should we instrument these at some point?
- // ink_hrtime cache_read_begin;
- // ink_hrtime cache_read_end;
- // ink_hrtime cache_write_begin;
- // ink_hrtime cache_write_end;
+ { // constructor to initialize array elements to zero in case user wants to
+ for (int i = 0; i < LAST_ENTRY; i++)
+ milestones[i] = 0;
+ }
+
+ void ms_set(Milestone ms, ink_hrtime val);
+ ink_hrtime ms_get(Milestone ms) const;
--- End diff --
`operator[]` ?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request: TS-2150: Add Milestone log tags
Posted by shinrich <gi...@git.apache.org>.
Github user shinrich commented on the pull request:
https://github.com/apache/trafficserver/pull/229#issuecomment-119236957
The changes look good to me. Please resolve the merge conflict, and I'll get this committed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request: TS-2150: Add Milestone log tags
Posted by SolidWallOfCode <gi...@git.apache.org>.
Github user SolidWallOfCode commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/229#discussion_r33062790
--- Diff: proxy/logging/LogField.cc ---
@@ -212,6 +269,51 @@ LogField::~LogField()
ats_free(m_symbol);
}
+TransactionMilestones::Milestone
+LogField::milestone_from_m_name(const char *m_name)
+{
+ milestone_map::iterator it;
+ TransactionMilestones::Milestone result = TransactionMilestones::LAST_ENTRY;
+
+ it = m_milestone_map.find(m_name);
+ if (it != m_milestone_map.end())
+ result = it->second;
+
+ return result;
+}
+
+int
+LogField::milestones_from_m_name(const char *m_name, TransactionMilestones::Milestone *ms1, TransactionMilestones::Milestone *ms2)
+{
+ milestone_map::iterator it;
+ char *ms_name;
+ size_t ms_name_len, skip;
+
+ ms_name_len = strcspn(m_name, " -");
+ ms_name = strndup(m_name, ms_name_len);
+ it = m_milestone_map.find(ms_name);
+ free(ms_name);
+ if (it != m_milestone_map.end()) {
+ *ms1 = it->second;
+ } else {
+ for (it = m_milestone_map.begin(); it != m_milestone_map.end(); ++it)
--- End diff --
What is this point of this loop, if it returns on the first iteration?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request: TS-2150: Add Milestone log tags
Posted by zwoop <gi...@git.apache.org>.
Github user zwoop commented on the pull request:
https://github.com/apache/trafficserver/pull/229#issuecomment-114597667
+1. No STL on the critical path.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---