KVL-914 Add unit test and fix trace context propagation from config management service #9694
Next
Next commit
Add unit test for trace context propagation from config management se…
…rvice Fix invalid telemetry context CHANGELOG_BEGIN CHANGELOG_END
- Loading branch information
commit d1a42912e3196d34d80007959cefe88f9a80e5df
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,124 @@ | ||
| package com.daml.platform.apiserver.services.admin | ||
|
|
||
| import java.time.{Duration => jDuration} | ||
| import java.util.concurrent.{CompletableFuture, CompletionStage} | ||
|
|
||
| import akka.stream.scaladsl.Source | ||
| import com.daml.api.util.TimeProvider | ||
| import com.daml.ledger.api.domain | ||
| import com.daml.ledger.api.domain.LedgerOffset.Absolute | ||
| import com.daml.ledger.api.testing.utils.AkkaBeforeAndAfterAll | ||
| import com.daml.ledger.api.v1.admin.config_management_service.{SetTimeModelRequest, TimeModel} | ||
| import com.daml.ledger.participant.state.index.v2.IndexConfigManagementService | ||
| import com.daml.ledger.participant.state.v1 | ||
| import com.daml.ledger.participant.state.v1.{ | ||
| Configuration, | ||
| SubmissionId, | ||
| SubmissionResult, | ||
| WriteConfigService, | ||
| } | ||
| import com.daml.lf.data.{Ref, Time} | ||
| import com.daml.logging.LoggingContext | ||
| import com.daml.platform.configuration.LedgerConfiguration | ||
| import com.daml.telemetry.{TelemetryContext, TelemetrySpecBase} | ||
| import com.google.protobuf.duration.Duration | ||
| import com.google.protobuf.timestamp.Timestamp | ||
| import org.mockito.{ArgumentMatchersSugar, MockitoSugar} | ||
| import org.scalatest.matchers.should.Matchers | ||
| import org.scalatest.wordspec.AsyncWordSpec | ||
|
|
||
| import scala.concurrent.Future | ||
|
|
||
| class ApiConfigManagementServiceSpec | ||
| extends AsyncWordSpec | ||
| with TelemetrySpecBase | ||
| with MockitoSugar | ||
| with Matchers | ||
| with ArgumentMatchersSugar | ||
| with AkkaBeforeAndAfterAll { | ||
|
|
||
| import ApiConfigManagementServiceSpec._ | ||
|
|
||
| private implicit val loggingContext: LoggingContext = LoggingContext.ForTesting | ||
|
|
||
| "ApiConfigManagementService" should { | ||
| "propagate trace context" in { | ||
| val mockReadBackend = mockIndexConfigManagementService() | ||
|
|
||
| val apiConfigManagementService = ApiConfigManagementService.createApiService( | ||
| mockReadBackend, | ||
| TestWriteConfigService, | ||
| TimeProvider.UTC, | ||
| LedgerConfiguration.defaultLocalLedger, | ||
| ) | ||
|
|
||
| val span = anEmptySpan() | ||
| val scope = span.makeCurrent() | ||
| apiConfigManagementService | ||
| .setTimeModel(aSetTimeModelRequest) | ||
| .andThen { case _ => | ||
| scope.close() | ||
| span.end() | ||
| } | ||
| .map { _ => | ||
| spanExporter.finishedSpanAttributes should contain(anApplicationIdSpanAttribute) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| private def mockIndexConfigManagementService() = { | ||
| val mockIndexConfigManagementService = mock[IndexConfigManagementService] | ||
| when(mockIndexConfigManagementService.lookupConfiguration()(any[LoggingContext])) | ||
| .thenReturn(Future.successful(None)) | ||
| when( | ||
| mockIndexConfigManagementService.configurationEntries(any[Option[Absolute]])( | ||
| any[LoggingContext] | ||
| ) | ||
| ).thenReturn(configurationEntries) | ||
| mockIndexConfigManagementService | ||
| } | ||
|
|
||
| private object TestWriteConfigService extends WriteConfigService { | ||
| override def submitConfiguration( | ||
| maxRecordTime: Time.Timestamp, | ||
| submissionId: SubmissionId, | ||
| config: Configuration, | ||
| )(implicit telemetryContext: TelemetryContext): CompletionStage[SubmissionResult] = { | ||
| telemetryContext.setAttribute( | ||
| anApplicationIdSpanAttribute._1, | ||
| anApplicationIdSpanAttribute._2, | ||
| ) | ||
| CompletableFuture.completedFuture(SubmissionResult.Acknowledged) | ||
| } | ||
| } | ||
| } | ||
|
Comment on lines
+72
to
+97
There was a problem hiding this comment. Not hugely important but, since you already have a twin object, it looks like you could move these two definitions to it. There was a problem hiding this comment. Sadly I can't because:
|
||
|
|
||
| object ApiConfigManagementServiceSpec { | ||
| private val aSubmissionId = "aSubmission" | ||
|
|
||
| private val aConfigurationGeneration = 0L | ||
|
|
||
| private val configurationEntries = Source.single( | ||
| Absolute(Ref.LedgerString.assertFromString("0")) -> domain.ConfigurationEntry.Accepted( | ||
| aSubmissionId, | ||
| Configuration( | ||
| aConfigurationGeneration, | ||
| v1.TimeModel.reasonableDefault, | ||
| jDuration.ZERO, | ||
| ), | ||
| ) | ||
| ) | ||
|
|
||
| private val aSetTimeModelRequest = SetTimeModelRequest( | ||
| aSubmissionId, | ||
| Some(Timestamp.defaultInstance), | ||
| aConfigurationGeneration, | ||
| Some( | ||
| TimeModel( | ||
| Some(Duration.defaultInstance), | ||
| Some(Duration.defaultInstance), | ||
| Some(Duration.defaultInstance), | ||
| ) | ||
| ), | ||
| ) | ||
| } | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the sake of curiosity: was this test working with the
NoOp?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The
ApiConfigManagementServiceusesDefaultTelemetryby default, so hard to tell, TBH. ThesetAttributemethod wouldn't work in theNoOp. Not sure if this answers your question.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or you're asking about the change from
KeyValueParticipantStateWriter? Then yes, as this test doesn't go that far.