Skip to content

Commit c00e4e0

Browse files
committed
Address Comments
1 parent 2e181bd commit c00e4e0

File tree

6 files changed

+96
-136
lines changed

6 files changed

+96
-136
lines changed

shell/agents/Microsoft.Azure.Agent/AzureAgent.cs

Lines changed: 14 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -36,15 +36,15 @@ 7. DO NOT include the placeholder summary when the commands contains no placehol
3636
""";
3737

3838
private int _turnsLeft;
39-
private CopilotResponse _copilotResponse;
39+
internal CopilotResponse _copilotResponse;
4040
private AgentSetting _setting;
4141

4242
private readonly string _instructions;
4343
private readonly StringBuilder _buffer;
4444
private readonly HttpClient _httpClient;
45-
private readonly ChatSession _chatSession;
45+
internal readonly ChatSession _chatSession;
4646
private readonly Dictionary<string, string> _valueStore;
47-
private MetricHelper _metricHelper;
47+
// private MetricHelper _metricHelper;
4848

4949
public AzureAgent()
5050
{
@@ -90,7 +90,6 @@ public void Initialize(AgentConfig config)
9090

9191
_turnsLeft = int.MaxValue;
9292
_setting = AgentSetting.LoadFromFile(SettingFile);
93-
_metricHelper = new MetricHelper(ChatSession.CONVERSATION_URL);
9493

9594
if (_setting is null)
9695
{
@@ -131,13 +130,14 @@ public void OnUserAction(UserActionPayload actionPayload) {
131130
IsUserFeedback = true;
132131
}
133132

134-
_metricHelper.LogTelemetry(
133+
MetricHelper.metricHelper.LogTelemetry(
135134
new AzTrace()
136135
{
137136
Command = actionPayload.Action.ToString(),
138-
CorrelationId = _chatSession.CorrelationId,
137+
ConversationId = _chatSession.ConversationId,
138+
ActivityId = _copilotResponse.ReplyToId,
139139
EventType = IsUserFeedback ? "Feedback" : "UserAction",
140-
Handler = _copilotResponse.TopicName,
140+
TopicName = _copilotResponse.TopicName,
141141
DetailedMessage = DetailedMessage
142142
});
143143
}
@@ -196,12 +196,12 @@ public async Task<bool> ChatAsync(string input, IShell shell)
196196

197197
if (!MetricHelper.TelemetryOptOut)
198198
{
199-
_metricHelper.LogTelemetry(
199+
MetricHelper.metricHelper.LogTelemetry(
200200
new AzTrace()
201201
{
202-
CorrelationId = _chatSession.CorrelationId,
202+
ConversationId = _chatSession.ConversationId,
203203
EventType = "Chat",
204-
Handler = _copilotResponse.TopicName,
204+
TopicName = _copilotResponse.TopicName,
205205
ActivityId = _copilotResponse.ReplyToId
206206
});
207207
}
@@ -364,12 +364,13 @@ private ResponseData ParseCLIHandlerResponse(IShell shell)
364364
// TODO: send telemetry about this case.
365365
if (!MetricHelper.TelemetryOptOut)
366366
{
367-
_metricHelper.LogTelemetry(
367+
MetricHelper.metricHelper.LogTelemetry(
368368
new AzTrace()
369369
{
370-
CorrelationId = _chatSession.CorrelationId,
370+
ConversationId = _chatSession.ConversationId,
371+
ActivityId = _copilotResponse.ReplyToId,
371372
EventType = "Exception",
372-
Handler = _copilotResponse.TopicName,
373+
TopicName = _copilotResponse.TopicName,
373374
DetailedMessage = "The placeholder section is not in the format as we've instructed"
374375
});
375376
}
@@ -430,7 +431,6 @@ internal void ReplaceKnownPlaceholders(ResponseData data)
430431
}
431432
}
432433

433-
Dictionary<string, Boolean> DetailedMessage = new();
434434
if (pairs.Count == placeholders.Count)
435435
{
436436
data.PlaceholderSet = null;
@@ -439,26 +439,8 @@ internal void ReplaceKnownPlaceholders(ResponseData data)
439439
{
440440
for (int i = indices.Count - 1; i >= 0; i--)
441441
{
442-
DetailedMessage.Add(placeholders[indices[i]].Name.Trim(new Char[] { '<', '>'}), true);
443442
placeholders.RemoveAt(indices[i]);
444443
}
445-
foreach (var i in placeholders)
446-
{
447-
DetailedMessage.Add(i.Name.Trim(new Char[] { '<', '>' }), false);
448-
}
449-
}
450-
451-
if (!MetricHelper.TelemetryOptOut)
452-
{
453-
_metricHelper.LogTelemetry(
454-
new AzTrace()
455-
{
456-
Command = "Replace",
457-
CorrelationId = _chatSession.CorrelationId,
458-
EventType = "UserAction",
459-
Handler = _copilotResponse.TopicName,
460-
DetailedMessage = JsonSerializer.Serialize(DetailedMessage)
461-
});
462444
}
463445
}
464446

shell/agents/Microsoft.Azure.Agent/ChatSession.cs

Lines changed: 1 addition & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@ internal class ChatSession : IDisposable
1919
private string _streamUrl;
2020
private string _conversationId;
2121
private string _conversationUrl;
22-
private string _correlationId;
2322
private DateTime _expireOn;
2423
private AzureCopilotReceiver _copilotReceiver;
2524

@@ -28,7 +27,6 @@ internal class ChatSession : IDisposable
2827
private readonly Dictionary<string, object> _flights;
2928

3029
internal string ConversationId => _conversationId;
31-
internal string CorrelationId => _correlationId;
3230

3331
internal ChatSession(HttpClient httpClient)
3432
{
@@ -96,19 +94,12 @@ private void Reset()
9694
_streamUrl = null;
9795
_conversationId = null;
9896
_conversationUrl = null;
99-
_correlationId = null;
10097
_expireOn = DateTime.MinValue;
10198

10299
_copilotReceiver?.Dispose();
103100
_copilotReceiver = null;
104101
}
105102

106-
private string NewCorrelationID()
107-
{
108-
_correlationId = Guid.NewGuid().ToString();
109-
return _correlationId;
110-
}
111-
112103
private async Task GenerateTokenAsync(IHost host, CancellationToken cancellationToken)
113104
{
114105
// TODO: use spinner when generating token. Also use interaction for authentication is needed.
@@ -248,8 +239,7 @@ private HttpRequestMessage PrepareForChat(string input)
248239
var request = new HttpRequestMessage(HttpMethod.Post, _conversationUrl) { Content = content };
249240

250241
request.Headers.Authorization = new AuthenticationHeaderValue("Bearer", _token);
251-
// These headers are for server side telemetry. We refresh correlation ID for each query.
252-
request.Headers.Add("CorrelationId", NewCorrelationID());
242+
// These header is for server side telemetry to identify where the request comes from.
253243
request.Headers.Add("ClientType", "AIShell");
254244
return request;
255245
}
@@ -319,7 +309,6 @@ internal async Task<CopilotResponse> GetChatResponseAsync(string input, IStatusC
319309
catch (OperationCanceledException)
320310
{
321311
// TODO: we may need to notify azure copilot somehow about the cancellation.
322-
// TODO: Decide whether to record the cancellation in Telemetry
323312
return null;
324313
}
325314
}

shell/agents/Microsoft.Azure.Agent/Command.cs

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
using System.CommandLine;
22
using System.Text;
3+
using System.Text.Json;
34
using AIShell.Abstraction;
45

56
namespace Microsoft.Azure.Agent;
@@ -67,6 +68,8 @@ private void ReplaceAction()
6768

6869
try
6970
{
71+
// Detailed Message recorded indicating whether each placeholder is replaced.
72+
Dictionary<string, Boolean> DetailedMessage = new();
7073
for (int i = 0; i < items.Count; i++)
7174
{
7275
var item = items[i];
@@ -117,17 +120,37 @@ private void ReplaceAction()
117120

118121
_values.Add(item.Name, value);
119122
_agent.SaveUserValue(item.Name, value);
123+
DetailedMessage.Add(item.Name.Trim(new Char[] { '<', '>' }), true);
120124

121125
if (nameArgInfo is not null && nameArgInfo.NamingRule.TryMatchName(value, out string prodName, out string envName))
122126
{
123127
_productNames.Add(prodName.ToLower());
124128
_environmentNames.Add(envName.ToLower());
125129
}
126130
}
131+
else
132+
{
133+
DetailedMessage.Add(item.Name.Trim(new Char[] { '<', '>' }), false);
134+
}
127135

128136
// Write an extra new line.
129137
host.WriteLine();
130138
}
139+
140+
// Send Telemetry for Replace Action.
141+
if (!MetricHelper.TelemetryOptOut)
142+
{
143+
MetricHelper.metricHelper.LogTelemetry(
144+
new AzTrace()
145+
{
146+
Command = "Replace",
147+
ConversationId = _agent._chatSession.ConversationId,
148+
ActivityId = _agent._copilotResponse.ReplyToId,
149+
EventType = "UserAction",
150+
TopicName = _agent._copilotResponse.TopicName,
151+
DetailedMessage = JsonSerializer.Serialize(DetailedMessage)
152+
});
153+
}
131154
}
132155
catch (OperationCanceledException)
133156
{

shell/agents/Microsoft.Azure.Agent/DataRetriever.cs

Lines changed: 36 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
using System.Text.RegularExpressions;
77

88
using AIShell.Abstraction;
9+
using Azure;
910
using Serilog;
1011

1112
namespace Microsoft.Azure.Agent;
@@ -577,13 +578,46 @@ private AzCLICommand QueryForMetadata(string azCommand)
577578
}
578579
else
579580
{
580-
// TODO: telemetry.
581+
if (!MetricHelper.TelemetryOptOut)
582+
{
583+
Dictionary<string, string> errorMessage = new Dictionary<string, string>
584+
{
585+
{ "StatusCode", response.StatusCode.ToString() },
586+
{ "Command", azCommand },
587+
{ "ErrorMessage", $"[QueryForMetadata] Received status code {response.StatusCode} for command {azCommand}" },
588+
};
589+
MetricHelper.metricHelper.LogTelemetry(
590+
new AzTrace()
591+
{
592+
// ConversationId = _agent._chatSession.ConversationId,
593+
// ActivityId = _agent._copilotResponse.ReplyToId,
594+
EventType = "Exception",
595+
// TopicName = _agent._copilotResponse.TopicName,
596+
DetailedMessage = JsonSerializer.Serialize(errorMessage)
597+
});
598+
}
581599
Log.Error("[QueryForMetadata] Received status code '{0}' for command '{1}'", response.StatusCode, azCommand);
582600
}
583601
}
584602
catch (Exception e)
585603
{
586-
// TODO: telemetry.
604+
if (!MetricHelper.TelemetryOptOut)
605+
{
606+
Dictionary<string, string> errorMessage = new Dictionary<string, string>
607+
{
608+
{ "Command", azCommand },
609+
{ "ErrorMessage", $"[QueryForMetadata] Exception while processing command: {azCommand}" },
610+
};
611+
MetricHelper.metricHelper.LogTelemetry(
612+
new AzTrace()
613+
{
614+
// ConversationId = _agent._chatSession.ConversationId,
615+
// ActivityId = _agent._copilotResponse.ReplyToId,
616+
EventType = "Exception",
617+
// TopicName = _agent._copilotResponse.TopicName,
618+
DetailedMessage = JsonSerializer.Serialize(errorMessage)
619+
});
620+
}
587621
Log.Error(e, "[QueryForMetadata] Exception while processing command: {0}", azCommand);
588622
}
589623

shell/agents/Microsoft.Azure.Agent/Telemetry/AzTrace.cs

Lines changed: 10 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
using System.Text.Json;
2+
using static Microsoft.ApplicationInsights.MetricDimensionNames.TelemetryContext;
23

34
namespace Microsoft.Azure.Agent;
45

@@ -7,8 +8,10 @@ public class AzTrace
78
private static readonly string s_installationId;
89
private static string GetInstallationID()
910
{
11+
string azureConfigDir = Environment.GetEnvironmentVariable("AZURE_CONFIG_DIR");
1012
string userProfile = Environment.GetFolderPath(Environment.SpecialFolder.UserProfile);
11-
string userProfilePath = Path.Combine(userProfile, ".Azure", "azureProfile.json");
13+
string userProfilePath = Path.Combine(string.IsNullOrEmpty(azureConfigDir) ? userProfile : azureConfigDir, "azureProfile.json");
14+
1215
FileStream jsonStream;
1316
JsonElement array;
1417
string installationId;
@@ -21,9 +24,9 @@ private static string GetInstallationID()
2124
}
2225
else
2326
{
24-
userProfilePath = Path.Combine(userProfile, ".Azure", "AzureRmContextSettings.json");
2527
try
2628
{
29+
Path.Combine(string.IsNullOrEmpty(azureConfigDir) ? userProfile : azureConfigDir, "azureProfile.json");
2730
jsonStream = new FileStream(userProfilePath, FileMode.Open, FileAccess.Read);
2831
array = JsonSerializer.Deserialize<JsonElement>(jsonStream);
2932
installationId = array.GetProperty("Settings").GetProperty("InstallationId").GetString();
@@ -38,9 +41,11 @@ private static string GetInstallationID()
3841
return installationId;
3942
}
4043

41-
public string Handler;
42-
// CorrelationId from client side.
43-
public string CorrelationId;
44+
public string TopicName;
45+
// Each chat has a unique conversationId. When the cx runs /refresh,
46+
// a new chat is initiated(i.e.a new conversationId will be created).
47+
public string ConversationId;
48+
// The activity id of the user's query
4449
public string ActivityId;
4550
public string InstallationId = s_installationId;
4651
public string EventType;
@@ -59,18 +64,3 @@ private static string GetInstallationID()
5964
public Dictionary<string, string> ExtendedProperties;
6065
static AzTrace() => s_installationId = GetInstallationID();
6166
}
62-
63-
// TODO: inherit from ChatMessage in PSSchema
64-
internal class HistoryMessage
65-
{
66-
internal HistoryMessage(string role, string content, string correlationId)
67-
{
68-
Role = role;
69-
Content = content;
70-
CorrelationId = correlationId;
71-
}
72-
73-
public string Role { get; }
74-
public string Content { get; }
75-
public string CorrelationId { get; }
76-
}

0 commit comments

Comments
 (0)