Skip to content

Commit

Permalink
Remove the telemetry for VT sequences (#15494)
Browse files Browse the repository at this point in the history
This removes the telemetry tracking which counted how many times each VT
sequence was executed, and how many times there were "failures". This
information isn't needed any more, and we were reaching the limit of how
many sequences we could track anyway.

Essentially what's been removed is the `TermTelemetry` class, but we are
still tracking some statemachine telemetry in the `ParserTracing` class.
And since that used the same trace logging provider as `TermTelemetry`,
I've now moved that definition into the `tracing.cpp` file. 

The code still compiles and runs without exploding.

Closes #15482
  • Loading branch information
j4james authored Jun 5, 2023
1 parent 8aefc7a commit e9de646
Show file tree
Hide file tree
Showing 15 changed files with 16 additions and 655 deletions.
37 changes: 0 additions & 37 deletions src/host/telemetry.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,6 @@ Telemetry::Telemetry() :
_rgiProcessFileNameIndex(),
_rguiProcessFileNamesCount(),
_rgiAlphabeticalIndex(),
_rguiProcessFileNamesCodesCount(),
_rguiProcessFileNamesFailedCodesCount(),
_rguiProcessFileNamesFailedOutsideCodesCount(),
_rguiTimesApiUsed(),
_rguiTimesApiUsedAnsi(),
_uiNumberProcessFileNames(0),
Expand Down Expand Up @@ -214,28 +211,6 @@ void Telemetry::FindDialogClosed()
_uiFindNextClickedTotal = 0;
}

// Total up all the used VT100 codes and assign them to the last process that was attached.
// We originally did this when each process disconnected, but some processes don't
// disconnect when the conhost process exits. So we have to remember the last process that connected.
void Telemetry::TotalCodesForPreviousProcess()
{
using namespace Microsoft::Console::VirtualTerminal;
// Get the values even if we aren't recording the previously connected process, since we want to reset them to 0.
auto _uiTimesUsedCurrent = TermTelemetry::Instance().GetAndResetTimesUsedCurrent();
auto _uiTimesFailedCurrent = TermTelemetry::Instance().GetAndResetTimesFailedCurrent();
auto _uiTimesFailedOutsideRangeCurrent = TermTelemetry::Instance().GetAndResetTimesFailedOutsideRangeCurrent();

if (_iProcessConnectedCurrently < c_iMaxProcessesConnected)
{
_rguiProcessFileNamesCodesCount[_iProcessConnectedCurrently] += _uiTimesUsedCurrent;
_rguiProcessFileNamesFailedCodesCount[_iProcessConnectedCurrently] += _uiTimesFailedCurrent;
_rguiProcessFileNamesFailedOutsideCodesCount[_iProcessConnectedCurrently] += _uiTimesFailedOutsideRangeCurrent;

// Don't total any more process connected telemetry, unless a new processes attaches that we want to gather.
_iProcessConnectedCurrently = SIZE_MAX;
}
}

// Tries to find the process name amongst our previous process names by doing a binary search.
// The main difference between this and the standard bsearch library call, is that if this
// can't find the string, it returns the position the new string should be inserted at. This saves
Expand Down Expand Up @@ -284,8 +259,6 @@ void Telemetry::LogProcessConnected(const HANDLE hProcess)
// This is a bit of processing, so don't do it for the 95% of machines that aren't being sampled.
if (TraceLoggingProviderEnabled(g_hConhostV2EventTraceProvider, 0, MICROSOFT_KEYWORD_MEASURES))
{
TotalCodesForPreviousProcess();

// Don't initialize wszFilePathAndName, QueryFullProcessImageName does that for us. Use QueryFullProcessImageName instead of
// GetProcessImageFileName because we need the path to begin with a drive letter and not a device name.
WCHAR wszFilePathAndName[MAX_PATH];
Expand Down Expand Up @@ -360,15 +333,8 @@ void Telemetry::WriteFinalTraceLog()
// This is a bit of processing, so don't do it for the 95% of machines that aren't being sampled.
if (TraceLoggingProviderEnabled(g_hConhostV2EventTraceProvider, 0, MICROSOFT_KEYWORD_MEASURES))
{
// Normally we would set the activity Id earlier, but since we know the parser only sends
// one final log at the end, setting the activity this late should be fine.
Microsoft::Console::VirtualTerminal::TermTelemetry::Instance().SetActivityId(_activity.Id());
Microsoft::Console::VirtualTerminal::TermTelemetry::Instance().SetShouldWriteFinalLog(_fUserInteractiveForTelemetry);

if (_fUserInteractiveForTelemetry)
{
TotalCodesForPreviousProcess();

// Send this back using "measures" since we want a good sampling of our entire userbase.
time_t tEndedAt;
time(&tEndedAt);
Expand All @@ -395,9 +361,6 @@ void Telemetry::WriteFinalTraceLog()
// Casting to UINT should be fine, since our array size is only 2K.
TraceLoggingPackedField(_wchProcessFileNames, static_cast<UINT>(sizeof(WCHAR) * _iProcessFileNamesNext), TlgInUNICODESTRING | TlgInVcount, "ProcessesConnected"),
TraceLoggingUInt32Array(_rguiProcessFileNamesCount, _uiNumberProcessFileNames, "ProcessesConnectedCount"),
TraceLoggingUInt32Array(_rguiProcessFileNamesCodesCount, _uiNumberProcessFileNames, "ProcessesConnectedCodesCount"),
TraceLoggingUInt32Array(_rguiProcessFileNamesFailedCodesCount, _uiNumberProcessFileNames, "ProcessesConnectedFailedCodesCount"),
TraceLoggingUInt32Array(_rguiProcessFileNamesFailedOutsideCodesCount, _uiNumberProcessFileNames, "ProcessesConnectedFailedOutsideCount"),
// Send back both starting and ending times separately instead just usage time (ending - starting).
// This can help us determine if they were using multiple consoles at the same time.
TraceLoggingInt32(static_cast<int>(_tStartedAt), "StartedUsingAtSeconds"),
Expand Down
7 changes: 0 additions & 7 deletions src/host/telemetry.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,6 @@ class Telemetry
void operator=(const Telemetry&);

bool FindProcessName(const WCHAR* pszProcessName, _Out_ size_t* iPosition) const;
void TotalCodesForPreviousProcess();

static const int c_iMaxProcessesConnected = 100;

Expand All @@ -159,12 +158,6 @@ class Telemetry
unsigned int _rguiProcessFileNamesCount[c_iMaxProcessesConnected];
// To speed up searching the Process Names, create an alphabetically sorted index.
size_t _rgiAlphabeticalIndex[c_iMaxProcessesConnected];
// Total of how many codes each process used
unsigned int _rguiProcessFileNamesCodesCount[c_iMaxProcessesConnected];
// Total of how many failed codes each process used
unsigned int _rguiProcessFileNamesFailedCodesCount[c_iMaxProcessesConnected];
// Total of how many failed codes each process used outside the valid range.
unsigned int _rguiProcessFileNamesFailedOutsideCodesCount[c_iMaxProcessesConnected];
unsigned int _rguiTimesApiUsed[NUMBER_OF_APIS];
// Most of this array will be empty, and is only used if an API has an ansi specific variant.
unsigned int _rguiTimesApiUsedAnsi[NUMBER_OF_APIS];
Expand Down
1 change: 0 additions & 1 deletion src/terminal/parser/InputStateMachineEngine.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ Author(s):
--*/
#pragma once

#include "telemetry.hpp"
#include "IStateMachineEngine.hpp"
#include <functional>
#include "../../types/inc/IInputEvent.hpp"
Expand Down
Loading

0 comments on commit e9de646

Please sign in to comment.