Skip to content

Commit

Permalink
Fixes from review:
Browse files Browse the repository at this point in the history
* Caching still doesn't work !?!?!
  • Loading branch information
johnml1135 committed Jan 12, 2024
1 parent 0c64439 commit cfb3391
Show file tree
Hide file tree
Showing 14 changed files with 46 additions and 50 deletions.
12 changes: 9 additions & 3 deletions samples/EchoTranslationEngine/TranslationEngineServiceV1.cs
Original file line number Diff line number Diff line change
@@ -1,10 +1,16 @@
namespace EchoTranslationEngine;

public class TranslationEngineServiceV1(BackgroundTaskQueue taskQueue, HealthCheckService healthCheckService) : TranslationEngineApi.TranslationEngineApiBase
public class TranslationEngineServiceV1 : TranslationEngineApi.TranslationEngineApiBase
{
private static readonly Empty Empty = new();
private readonly BackgroundTaskQueue _taskQueue = taskQueue;
private readonly HealthCheckService _healthCheckService = healthCheckService;
private readonly BackgroundTaskQueue _taskQueue;
private readonly HealthCheckService _healthCheckService;

public TranslationEngineServiceV1(BackgroundTaskQueue taskQueue, HealthCheckService healthCheckService)
{
_taskQueue = taskQueue;
_healthCheckService = healthCheckService;
}

public override Task<Empty> Create(CreateRequest request, ServerCallContext context)
{
Expand Down
8 changes: 4 additions & 4 deletions src/Serval.ApiServer/Controllers/StatusController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ IConfiguration configuration
/// <response code="403">The authenticated client cannot perform the operation</response>
[Authorize(Scopes.ReadStatus)]
[HttpGet("health")]
[OutputCache(PolicyName = "CacheHealthStatus")]
[OutputCache]
[ProducesResponseType(typeof(HealthReportDto), (int)HttpStatusCode.OK)]
[ProducesResponseType(typeof(void), StatusCodes.Status401Unauthorized)]
[ProducesResponseType(typeof(void), StatusCodes.Status403Forbidden)]
Expand All @@ -47,10 +47,10 @@ public async Task<ActionResult<HealthReportDto>> GetHealthAsync()
/// </summary>
/// <remarks>Provides an indication about the health of the API</remarks>
/// <response code="200">The API health status</response>
[HttpGet("health-public")]
[OutputCache(PolicyName = "CacheHealthStatus")]
[HttpGet("ping")]
[OutputCache]
[ProducesResponseType(typeof(HealthReportDto), (int)HttpStatusCode.OK)]
public async Task<ActionResult<HealthReportDto>> GetPublicHealthAsync()
public async Task<ActionResult<HealthReportDto>> GetPingAsync()
{
var report = await _healthCheckService.CheckHealthAsync();
HealthReportDto reportDto = Map(report);
Expand Down
16 changes: 7 additions & 9 deletions src/Serval.ApiServer/Startup.cs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ public void ConfigureServices(IServiceCollection services)
{
services.AddRouting(o => o.LowercaseUrls = true);

services.AddOutputCache();

services
.AddControllers()
.AddJsonOptions(o =>
Expand All @@ -42,9 +44,10 @@ public void ConfigureServices(IServiceCollection services)
});
services.AddHealthChecks().AddIdentityServer(new Uri(authority), name: "Auth0");

string DataFilesDir = Configuration!.GetSection(DataFileOptions.Key)!.GetValue<string>("FilesDirectory")!;
var dataFileOptions = new DataFileOptions();
Configuration.GetSection(DataFileOptions.Key).Bind(dataFileOptions);
// find drive letter for DataFilesDir
string driveLetter = Path.GetPathRoot(DataFilesDir)![..1];
string driveLetter = Path.GetPathRoot(dataFileOptions.FilesDirectory)![..1];

// add health check for disk storage capacity
services
Expand All @@ -55,11 +58,6 @@ public void ConfigureServices(IServiceCollection services)
HealthStatus.Degraded
);

services.AddOutputCache(options =>
{
options.AddPolicy("CacheHealthStatus", builder => builder.Expire(TimeSpan.FromSeconds(10)));
});

services.AddAuthorization(o =>
{
foreach (string scope in Scopes.All)
Expand Down Expand Up @@ -206,16 +204,16 @@ public void Configure(IApplicationBuilder app, IWebHostEnvironment env)

app.UseRouting();
app.UseAuthorization();
app.UseOutputCache();
app.UseEndpoints(x =>
{
x.MapControllers();
x.MapServalTranslationServices();
x.MapHangfireDashboard();
x.MapHealthChecks("/health", new HealthCheckOptions { ResponseWriter = WriteHealthCheckResponse.Generate });
;
});

app.UseOutputCache();

app.UseOpenApi(o =>
{
o.PostProcess = (document, request) =>
Expand Down
3 changes: 2 additions & 1 deletion src/Serval.ApiServer/Usings.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
global using System.Net;
global using System.Security.Claims;
global using System.Text;
global using System.Text.Json;
global using System.Text.Json.Serialization;
global using Asp.Versioning;
global using Hangfire;
Expand Down Expand Up @@ -30,4 +32,3 @@
global using Serval.Shared.Controllers;
global using Serval.Shared.Models;
global using Serval.Shared.Services;
global using Serval.Shared.Utils;
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
namespace Serval.Shared.Utils;
namespace Serval.ApiServer;

public class WriteHealthCheckResponse
{
Expand Down
6 changes: 3 additions & 3 deletions src/Serval.Client/Client.g.cs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ public partial interface IStatusClient

/// <param name="cancellationToken">A cancellation token that can be used by other objects or threads to receive notice of cancellation.</param>
/// <exception cref="ServalApiException">A server side error occurred.</exception>
System.Threading.Tasks.Task<HealthReport> GetPublicHealthAsync(System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken));
System.Threading.Tasks.Task<HealthReport> GetPingAsync(System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken));

/// <param name="cancellationToken">A cancellation token that can be used by other objects or threads to receive notice of cancellation.</param>
/// <exception cref="ServalApiException">A server side error occurred.</exception>
Expand Down Expand Up @@ -153,10 +153,10 @@ public string BaseUrl

/// <param name="cancellationToken">A cancellation token that can be used by other objects or threads to receive notice of cancellation.</param>
/// <exception cref="ServalApiException">A server side error occurred.</exception>
public virtual async System.Threading.Tasks.Task<HealthReport> GetPublicHealthAsync(System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken))
public virtual async System.Threading.Tasks.Task<HealthReport> GetPingAsync(System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken))
{
var urlBuilder_ = new System.Text.StringBuilder();
urlBuilder_.Append(BaseUrl != null ? BaseUrl.TrimEnd('/') : "").Append("/status/health-public");
urlBuilder_.Append(BaseUrl != null ? BaseUrl.TrimEnd('/') : "").Append("/status/ping");

var client_ = _httpClient;
var disposeClient_ = false;
Expand Down
13 changes: 6 additions & 7 deletions src/Serval.Grpc/Protos/serval/translation/v1/engine.proto
Original file line number Diff line number Diff line change
Expand Up @@ -105,10 +105,9 @@ message TranslationResult {
}

message HealthCheckResponse {
HealthCheckStatus Status = 1;
string Duration = 2;
map<string, string> Data = 3;
optional string Exception = 4;
HealthCheckStatus status = 1;
map<string, string> data = 2;
optional string error = 3;
}

message WordGraphArc {
Expand Down Expand Up @@ -160,7 +159,7 @@ enum TranslationSource {
}

enum HealthCheckStatus {
Unhealthy = 0;
Degraded = 1;
Healthy = 2;
UNHEALTHY = 0;
DEGRADED = 1;
HEALTHY = 2;
}
8 changes: 2 additions & 6 deletions src/Serval.Grpc/Utils/WriteGrpcHealthCheckResponse.cs
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,8 @@ public static HealthCheckResponse Generate(HealthReport healthReport)
healthCheckResultException += $"\n{entry.Key}: {entry.Value.Exception}";
}
// Assemble response
HealthCheckResponse healthCheckReponse = new HealthCheckResponse
{
Status = (HealthCheckStatus)healthReport.Status,
Duration = healthReport.TotalDuration.ToString(),
Exception = healthCheckResultException ?? ""
};
HealthCheckResponse healthCheckReponse =
new() { Status = (HealthCheckStatus)healthReport.Status, Error = healthCheckResultException ?? "" };
foreach (KeyValuePair<string, string> entry in healthCheckResultData)
{
healthCheckReponse.Data.Add(entry.Key, entry.Value);
Expand Down
9 changes: 5 additions & 4 deletions src/Serval.Shared/Configuration/IServalBuilderExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,11 @@ public static IServalBuilder AddMongoDataAccess(
Action<IMongoDataAccessConfigurator> configure
)
{
builder.Services.AddMongoDataAccess(builder.Configuration!.GetConnectionString("Mongo")!, "Serval", configure);
builder
.Services.AddHealthChecks()
.AddMongoDb(builder.Configuration!.GetConnectionString("Mongo")!, name: "Mongo");
var mongoConnectionString =
builder.Configuration?.GetConnectionString("Mongo")
?? throw new Exception("Mongo connection string not configured");
builder.Services.AddMongoDataAccess(mongoConnectionString, "Serval", configure);
builder.Services.AddHealthChecks().AddMongoDb(mongoConnectionString, name: "Mongo");
return builder;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,7 @@ public override void OnException(ExceptionContext context)
|| (context.Exception is RpcException rpcEx && rpcEx.StatusCode == StatusCode.Unavailable)
)
{
_logger.Log(
LogLevel.Error,
"A user tried to access an unavailable service. Exception: \n{Exception}",
context.Exception
);
_logger.Log(LogLevel.Error, context.Exception, "A user tried to access an unavailable service.");
context.Result = new StatusCodeResult(StatusCodes.Status503ServiceUnavailable);
context.ExceptionHandled = true;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,7 @@ public static IServalBuilder AddTranslation(
builder.Services.AddScoped<IEngineService, EngineService>();

var translationOptions = new TranslationOptions();
if (builder.Configuration is not null)
builder.Configuration.GetSection(TranslationOptions.Key).Bind(translationOptions);
builder.Configuration?.GetSection(TranslationOptions.Key).Bind(translationOptions);
if (configure is not null)
configure(translationOptions);

Expand Down
10 changes: 5 additions & 5 deletions src/Serval.Translation/Services/GrpcServiceHealthCheck.cs
Original file line number Diff line number Diff line change
Expand Up @@ -20,21 +20,21 @@ public async Task<HealthCheckResult> CheckHealthAsync(
var client = _grpcClientFactory.CreateClient<TranslationEngineApi.TranslationEngineApiClient>(
context.Registration.Name
);
HealthCheckResponse? healthReport = await client.HealthCheckAsync(
HealthCheckResponse? healthCheckResponse = await client.HealthCheckAsync(
new Google.Protobuf.WellKnownTypes.Empty(),
cancellationToken: cancellationToken
);
if (healthReport is null)
if (healthCheckResponse is null)
return HealthCheckResult.Unhealthy(
$"Health check for {context.Registration.Name} failed with response null"
);
// map health report to health check result
HealthCheckResult healthCheckResult =
new(
status: (HealthStatus)healthReport.Status,
status: (HealthStatus)healthCheckResponse.Status,
description: context.Registration.Name,
exception: healthReport.Exception is null ? null : new Exception(healthReport.Exception),
data: healthReport.Data.ToDictionary(kvp => kvp.Key, kvp => (object)kvp.Value)
exception: healthCheckResponse.Error is null ? null : new Exception(healthCheckResponse.Error),
data: healthCheckResponse.Data.ToDictionary(kvp => kvp.Key, kvp => (object)kvp.Value)
);
return healthCheckResult;
}
Expand Down

0 comments on commit cfb3391

Please sign in to comment.