Skip to content
This repository was archived by the owner on Nov 1, 2023. It is now read-only.

Commit 7b79dd6

Browse files
chkeitachkeita
committed
Ignore regression update when the work item is in some states (#3532)
* Ignore regression update when the work item is in some states * format * formatting * don't hide messages in the poison queue * fix typo * update regression logic update test_template to support regression * build fix * mypy fix * build fix * move regression ignore state under ADODuplicateTemplate * replace extend with append * update set_tcp_keepalive * mke mypy happy * copy ADODuplicateTemplate.OnDuplicate.RegressionIgnoreStates
1 parent e10d8ea commit 7b79dd6

File tree

10 files changed

+132
-55
lines changed

10 files changed

+132
-55
lines changed

‎docs/webhook_events.md

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2033,6 +2033,10 @@ If webhook is set to have Event Grid message format then the payload will look a
20332033
},
20342034
"original_crash_test_result": {
20352035
"$ref": "#/definitions/CrashTestResult"
2036+
},
2037+
"report_url": {
2038+
"title": "Report Url",
2039+
"type": "string"
20362040
}
20372041
},
20382042
"required": [
@@ -6427,6 +6431,10 @@ If webhook is set to have Event Grid message format then the payload will look a
64276431
},
64286432
"original_crash_test_result": {
64296433
"$ref": "#/definitions/CrashTestResult"
6434+
},
6435+
"report_url": {
6436+
"title": "Report Url",
6437+
"type": "string"
64306438
}
64316439
},
64326440
"required": [

‎src/ApiService/ApiService/Functions/QueueFileChanges.cs

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -128,20 +128,22 @@ private async Async.Task RequeueMessage(string msg, TimeSpan? visibilityTimeout
128128
newCustomDequeueCount = json["data"]!["customDequeueCount"]!.GetValue<int>();
129129
}
130130

131-
var queueName = QueueFileChangesQueueName;
132131
if (newCustomDequeueCount > MAX_DEQUEUE_COUNT) {
133132
_log.LogWarning("Message retried more than {MAX_DEQUEUE_COUNT} times with no success: {msg}", MAX_DEQUEUE_COUNT, msg);
134-
queueName = QueueFileChangesPoisonQueueName;
133+
await _context.Queue.QueueObject(
134+
QueueFileChangesPoisonQueueName,
135+
json,
136+
StorageType.Config)
137+
.IgnoreResult();
138+
} else {
139+
json!["data"]!["customDequeueCount"] = newCustomDequeueCount + 1;
140+
await _context.Queue.QueueObject(
141+
QueueFileChangesQueueName,
142+
json,
143+
StorageType.Config,
144+
visibilityTimeout ?? CalculateExponentialBackoff(newCustomDequeueCount))
145+
.IgnoreResult();
135146
}
136-
137-
json!["data"]!["customDequeueCount"] = newCustomDequeueCount + 1;
138-
139-
await _context.Queue.QueueObject(
140-
queueName,
141-
json,
142-
StorageType.Config,
143-
visibilityTimeout ?? CalculateExponentialBackoff(newCustomDequeueCount))
144-
.IgnoreResult();
145147
}
146148

147149
// Possible return values:

‎src/ApiService/ApiService/OneFuzzTypes/Model.cs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -678,7 +678,8 @@ public record ADODuplicateTemplate(
678678
Dictionary<string, string> SetState,
679679
Dictionary<string, string> AdoFields,
680680
string? Comment = null,
681-
List<Dictionary<string, string>>? Unless = null
681+
List<Dictionary<string, string>>? Unless = null,
682+
List<string>? RegressionIgnoreStates = null
682683
);
683684

684685
public record AdoTemplate(
@@ -707,7 +708,7 @@ public record RenderedAdoTemplate(
707708
ADODuplicateTemplate OnDuplicate,
708709
Dictionary<string, string>? AdoDuplicateFields = null,
709710
string? Comment = null
710-
) : AdoTemplate(BaseUrl, AuthToken, Project, Type, UniqueFields, AdoFields, OnDuplicate, AdoDuplicateFields, Comment);
711+
) : AdoTemplate(BaseUrl, AuthToken, Project, Type, UniqueFields, AdoFields, OnDuplicate, AdoDuplicateFields, Comment) { }
711712

712713
public record TeamsTemplate(SecretData<string> Url) : NotificationTemplate {
713714
public Task<OneFuzzResultVoid> Validate() {

‎src/ApiService/ApiService/OneFuzzTypes/Requests.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,7 @@ public record NotificationSearch(
131131

132132

133133
public record NotificationTest(
134-
[property: Required] Report Report,
134+
[property: Required] IReport Report,
135135
[property: Required] Notification Notification
136136
) : BaseRequest;
137137

‎src/ApiService/ApiService/onefuzzlib/Reports.cs

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,10 @@
11
using System.Text.Json;
2+
using System.Text.Json.Serialization;
23
using System.Threading.Tasks;
34
using Microsoft.Extensions.Logging;
45
using Microsoft.OneFuzz.Service.OneFuzzLib.Orm;
6+
7+
58
namespace Microsoft.OneFuzz.Service;
69

710
public interface IReports {
@@ -85,6 +88,7 @@ public static IReport ParseReportOrRegression(string content, Uri reportUrl) {
8588
}
8689
}
8790

91+
[JsonConverter(typeof(ReportConverter))]
8892
public interface IReport {
8993
Uri? ReportUrl {
9094
init;
@@ -95,3 +99,19 @@ public string FileName() {
9599
return string.Concat(segments);
96100
}
97101
};
102+
103+
public class ReportConverter : JsonConverter<IReport> {
104+
105+
public override IReport? Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) {
106+
using var templateJson = JsonDocument.ParseValue(ref reader);
107+
108+
if (templateJson.RootElement.TryGetProperty("crash_test_result", out _)) {
109+
return templateJson.Deserialize<RegressionReport>(options);
110+
}
111+
return templateJson.Deserialize<Report>(options);
112+
}
113+
114+
public override void Write(Utf8JsonWriter writer, IReport value, JsonSerializerOptions options) {
115+
throw new NotImplementedException();
116+
}
117+
}

‎src/ApiService/ApiService/onefuzzlib/notifications/Ado.cs

Lines changed: 39 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ public class Ado : NotificationsBase, IAdo {
1919
// https://github.com/MicrosoftDocs/azure-devops-docs/issues/5890#issuecomment-539632059
2020
private const int MAX_SYSTEM_TITLE_LENGTH = 128;
2121
private const string TITLE_FIELD = "System.Title";
22+
private static List<string> DEFAULT_REGRESSION_IGNORE_STATES = new() { "New", "Commited", "Active" };
2223

2324
public Ado(ILogger<Ado> logTracer, IOnefuzzContext context) : base(logTracer, context) {
2425
}
@@ -56,7 +57,7 @@ public async Async.Task<OneFuzzResultVoid> NotifyAdo(AdoTemplate config, Contain
5657
_logTracer.LogEvent(adoEventType);
5758

5859
try {
59-
await ProcessNotification(_context, container, filename, config, report, _logTracer, notificationInfo);
60+
await ProcessNotification(_context, container, filename, config, report, _logTracer, notificationInfo, isRegression: reportable is RegressionReport);
6061
} catch (Exception e)
6162
when (e is VssUnauthorizedException || e is VssAuthenticationException || e is VssServiceException) {
6263
if (config.AdoFields.TryGetValue("System.AssignedTo", out var assignedTo)) {
@@ -298,7 +299,7 @@ private static async Async.Task<Dictionary<string, WorkItemField2>> GetValidFiel
298299
.ToDictionary(field => field.ReferenceName.ToLowerInvariant());
299300
}
300301

301-
private static async Async.Task ProcessNotification(IOnefuzzContext context, Container container, string filename, AdoTemplate config, Report report, ILogger logTracer, IList<(string, string)> notificationInfo, Renderer? renderer = null) {
302+
private static async Async.Task ProcessNotification(IOnefuzzContext context, Container container, string filename, AdoTemplate config, Report report, ILogger logTracer, IList<(string, string)> notificationInfo, Renderer? renderer = null, bool isRegression = false) {
302303
if (!config.AdoFields.TryGetValue(TITLE_FIELD, out var issueTitle)) {
303304
issueTitle = "{{ report.crash_site }} - {{ report.executable }}";
304305
}
@@ -311,7 +312,7 @@ private static async Async.Task ProcessNotification(IOnefuzzContext context, Con
311312

312313
var renderedConfig = RenderAdoTemplate(logTracer, renderer, config, instanceUrl);
313314
var ado = new AdoConnector(renderedConfig, project!, client, instanceUrl, logTracer, await GetValidFields(client, project));
314-
await ado.Process(notificationInfo);
315+
await ado.Process(notificationInfo, isRegression);
315316
}
316317

317318
public static RenderedAdoTemplate RenderAdoTemplate(ILogger logTracer, Renderer renderer, AdoTemplate original, Uri instanceUrl) {
@@ -352,7 +353,8 @@ public static RenderedAdoTemplate RenderAdoTemplate(ILogger logTracer, Renderer
352353
original.OnDuplicate.SetState,
353354
onDuplicateAdoFields,
354355
original.OnDuplicate.Comment != null ? Render(renderer, original.OnDuplicate.Comment, instanceUrl, logTracer) : null,
355-
onDuplicateUnless
356+
onDuplicateUnless,
357+
original.OnDuplicate.RegressionIgnoreStates
356358
);
357359

358360
return new RenderedAdoTemplate(
@@ -598,7 +600,7 @@ private async Async.Task<WorkItem> CreateNew() {
598600
return (taskType, document);
599601
}
600602

601-
public async Async.Task Process(IList<(string, string)> notificationInfo) {
603+
public async Async.Task Process(IList<(string, string)> notificationInfo, bool isRegression) {
602604
var updated = false;
603605
WorkItem? oldestWorkItem = null;
604606
await foreach (var workItem in ExistingWorkItems(notificationInfo)) {
@@ -612,6 +614,13 @@ public async Async.Task Process(IList<(string, string)> notificationInfo) {
612614
continue;
613615
}
614616

617+
var regressionStatesToIgnore = _config.OnDuplicate.RegressionIgnoreStates != null ? _config.OnDuplicate.RegressionIgnoreStates : DEFAULT_REGRESSION_IGNORE_STATES;
618+
if (isRegression) {
619+
var state = (string)workItem.Fields["System.State"];
620+
if (regressionStatesToIgnore.Contains(state, StringComparer.InvariantCultureIgnoreCase))
621+
continue;
622+
}
623+
615624
using (_logTracer.BeginScope("Non-duplicate work item")) {
616625
_logTracer.AddTags(new List<(string, string)> { ("NonDuplicateWorkItemId", $"{workItem.Id}") });
617626
_logTracer.LogInformation("Found matching non-duplicate work item");
@@ -621,30 +630,32 @@ public async Async.Task Process(IList<(string, string)> notificationInfo) {
621630
updated = true;
622631
}
623632

624-
if (!updated) {
625-
if (oldestWorkItem != null) {
626-
// We have matching work items but all are duplicates
627-
_logTracer.AddTags(notificationInfo);
628-
_logTracer.LogInformation($"All matching work items were duplicates, re-opening the oldest one");
629-
var stateChanged = await UpdateExisting(oldestWorkItem, notificationInfo);
630-
if (stateChanged) {
631-
// add a comment if we re-opened the bug
632-
_ = await _client.AddCommentAsync(
633-
new CommentCreate() {
634-
Text =
635-
"This work item was re-opened because OneFuzz could only find related work items that are marked as duplicate."
636-
},
637-
_project,
638-
(int)oldestWorkItem.Id!);
639-
}
640-
} else {
641-
// We never saw a work item like this before, it must be new
642-
var entry = await CreateNew();
643-
var adoEventType = "AdoNewItem";
644-
_logTracer.AddTags(notificationInfo);
645-
_logTracer.AddTag("WorkItemId", entry.Id.HasValue ? entry.Id.Value.ToString() : "");
646-
_logTracer.LogEvent(adoEventType);
633+
if (updated || isRegression) {
634+
return;
635+
}
636+
637+
if (oldestWorkItem != null) {
638+
// We have matching work items but all are duplicates
639+
_logTracer.AddTags(notificationInfo);
640+
_logTracer.LogInformation($"All matching work items were duplicates, re-opening the oldest one");
641+
var stateChanged = await UpdateExisting(oldestWorkItem, notificationInfo);
642+
if (stateChanged) {
643+
// add a comment if we re-opened the bug
644+
_ = await _client.AddCommentAsync(
645+
new CommentCreate() {
646+
Text =
647+
"This work item was re-opened because OneFuzz could only find related work items that are marked as duplicate."
648+
},
649+
_project,
650+
(int)oldestWorkItem.Id!);
647651
}
652+
} else {
653+
// We never saw a work item like this before, it must be new
654+
var entry = await CreateNew();
655+
var adoEventType = "AdoNewItem";
656+
_logTracer.AddTags(notificationInfo);
657+
_logTracer.AddTag("WorkItemId", entry.Id.HasValue ? entry.Id.Value.ToString() : "");
658+
_logTracer.LogEvent(adoEventType);
648659
}
649660
}
650661

‎src/cli/onefuzz/cli.py

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
Type,
2929
TypeVar,
3030
Union,
31+
cast,
3132
)
3233
from uuid import UUID
3334

@@ -551,8 +552,16 @@ def set_tcp_keepalive() -> None:
551552
# Azure Load Balancer default timeout (4 minutes)
552553
#
553554
# https://urllib3.readthedocs.io/en/stable/reference/urllib3.connection.html?highlight=keep-alive#:~:text=For%20example%2C%20if,socket.SO_KEEPALIVE%2C%201)%2C%0A%5D
554-
if value not in urllib3.connection.HTTPConnection.default_socket_options:
555-
urllib3.connection.HTTPConnection.default_socket_options.extend((value,))
555+
556+
default_socket_options = cast(
557+
List[Tuple[int, int, int]],
558+
urllib3.connection.HTTPConnection.default_socket_options,
559+
)
560+
561+
if value not in default_socket_options:
562+
default_socket_options + [
563+
value,
564+
]
556565

557566

558567
def execute_api(api: Any, api_types: List[Any], version: str) -> int:

‎src/cli/onefuzz/debug.py

Lines changed: 32 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,15 @@
2121
from azure.storage.blob import ContainerClient
2222
from onefuzztypes import models, requests, responses
2323
from onefuzztypes.enums import ContainerType, TaskType
24-
from onefuzztypes.models import BlobRef, Job, NodeAssignment, Report, Task, TaskConfig
24+
from onefuzztypes.models import (
25+
BlobRef,
26+
Job,
27+
NodeAssignment,
28+
RegressionReport,
29+
Report,
30+
Task,
31+
TaskConfig,
32+
)
2533
from onefuzztypes.primitives import Container, Directory, PoolName
2634
from onefuzztypes.responses import TemplateValidationResponse
2735

@@ -633,35 +641,50 @@ def test_template(
633641
self,
634642
notificationConfig: models.NotificationConfig,
635643
task_id: Optional[UUID] = None,
636-
report: Optional[Report] = None,
644+
report: Optional[str] = None,
637645
) -> responses.NotificationTestResponse:
638646
"""Test a notification template"""
639647

648+
the_report: Union[Report, RegressionReport, None] = None
649+
650+
if report is not None:
651+
try:
652+
the_report = RegressionReport.parse_raw(report)
653+
print("testing regression report")
654+
except Exception:
655+
the_report = Report.parse_raw(report)
656+
print("testing normal report")
657+
640658
if task_id is not None:
641659
task = self.onefuzz.tasks.get(task_id)
642-
if report is None:
660+
if the_report is None:
643661
input_blob_ref = BlobRef(
644662
account="dummy-storage-account",
645663
container="test-notification-crashes",
646664
name="fake-crash-sample",
647665
)
648-
report = self._create_report(
666+
the_report = self._create_report(
649667
task.job_id, task.task_id, "fake_target.exe", input_blob_ref
650668
)
669+
elif isinstance(the_report, RegressionReport):
670+
if the_report.crash_test_result.crash_report is None:
671+
raise Exception("invalid regression report: no crash report")
672+
the_report.crash_test_result.crash_report.task_id = task.task_id
673+
the_report.crash_test_result.crash_report.job_id = task.job_id
651674
else:
652-
report.task_id = task.task_id
653-
report.job_id = task.job_id
654-
elif report is None:
675+
the_report.task_id = task.task_id
676+
the_report.job_id = task.job_id
677+
elif the_report is None:
655678
raise Exception("must specify either task_id or report")
656679

657-
report.report_url = "https://dummy-container.blob.core.windows.net/dummy-reports/dummy-report.json"
680+
the_report.report_url = "https://dummy-container.blob.core.windows.net/dummy-reports/dummy-report.json"
658681

659682
endpoint = Endpoint(self.onefuzz)
660683
return endpoint._req_model(
661684
"POST",
662685
responses.NotificationTestResponse,
663686
data=requests.NotificationTest(
664-
report=report,
687+
report=the_report,
665688
notification=models.Notification(
666689
container=Container("test-notification-reports"),
667690
notification_id=uuid.uuid4(),

‎src/pytypes/onefuzztypes/models.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -256,13 +256,15 @@ class CrashTestResult(BaseModel):
256256
class RegressionReport(BaseModel):
257257
crash_test_result: CrashTestResult
258258
original_crash_test_result: Optional[CrashTestResult]
259+
report_url: Optional[str]
259260

260261

261262
class ADODuplicateTemplate(BaseModel):
262263
increment: List[str]
263264
comment: Optional[str]
264265
set_state: Dict[str, str]
265266
ado_fields: Dict[str, str]
267+
regression_ignore_states: Optional[List[str]]
266268

267269

268270
class ADOTemplate(BaseModel):

‎src/pytypes/onefuzztypes/requests.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
# Copyright (c) Microsoft Corporation.
44
# Licensed under the MIT License.
55

6-
from typing import Any, Dict, List, Optional
6+
from typing import Any, Dict, List, Optional, Union
77
from uuid import UUID
88

99
from pydantic import AnyHttpUrl, BaseModel, Field, root_validator
@@ -26,6 +26,7 @@
2626
AutoScaleConfig,
2727
InstanceConfig,
2828
NotificationConfig,
29+
RegressionReport,
2930
Report,
3031
TemplateRenderContext,
3132
)
@@ -280,7 +281,7 @@ class EventsGet(BaseModel):
280281

281282

282283
class NotificationTest(BaseModel):
283-
report: Report
284+
report: Union[Report, RegressionReport]
284285
notification: models.Notification
285286

286287

0 commit comments

Comments
 (0)