-
Notifications
You must be signed in to change notification settings - Fork 27
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Replayer instrumentation #475
Merged
gregschohn
merged 100 commits into
opensearch-project:main
from
gregschohn:ReplayerInstrumentation
Feb 9, 2024
Merged
Changes from 73 commits
Commits
Show all changes
100 commits
Select commit
Hold shift + click to select a range
a4caca7
Move addMetricsIfPresent into the metrics builder as a first class me…
gregschohn c026588
WIP to play with OpenTelemetry metric instruments and tracer spans.
gregschohn f3c0077
Get gradle files and docker-compose in order to support otlp exports …
gregschohn 7fb8e2e
WIP
gregschohn a8ae3d1
Restore the docker-compose single-node/multi-node split docker-compos…
gregschohn da9d36b
Add labels to each metric instrument so that multiple values can be p…
gregschohn 06618ca
Move the MetricsClosure into its own class and stop stuffing the metr…
gregschohn aba1aab
WIP - Cleanup + get Jaeger to work by switching the endpoint. Also i…
gregschohn 900bc6d
Start moving away from ThreadLocal and 'current contexts' and toward …
gregschohn 3746a8e
Get span parenting to work.
gregschohn e0e7bf1
Merge branch 'main' into DoNotMerge_MoreMetrics
gregschohn 4b43262
Attempt to fix a failing unit test.
gregschohn 322e12f
Refactor. Couple name changes, class package changes, and moved IRep…
gregschohn 723bf77
Bundle all of the offloader spans with the netty handler spans.
gregschohn 15a1705
Improve the tracing story for the capture proxy.
gregschohn 8a6f52a
Tracing change: Flatten the flush span and just record it as 'blocked'.
gregschohn c50e01d
Minor cleanup - stop setting the namespace or trying to change in a p…
gregschohn 17c517d
Start instrumenting the replayer with more contexts so that traces an…
gregschohn 6288844
Double down on using Context objects in lieu of String labels and fix…
gregschohn 09e849c
Merge branch 'FixKafkaResume' into OtelMetricsAndTraces
gregschohn 9cf2540
Merge branch 'main' into OtelMetricsAndTraces
gregschohn c14da6a
Update the Http Logging Handler to suppress response packet captures …
gregschohn 7de4009
File rename since the LoggingHttpRequest handler now handles both req…
gregschohn a5bfc7d
Shuffling lots of details of Contexts and the relationships between d…
gregschohn 3d60106
Lots of refactoring to get a couple more test cases to pass.
gregschohn ad6bd13
Test fixes
gregschohn 07ae016
Begin to cleanup endspans for some of the contexts. Lots of bugs rem…
gregschohn ccb517c
More work to get context chains to work better together.
gregschohn 8bda2e3
Two critical bugfixes around handling close observations that were di…
gregschohn d9df3fa
Fix some test code where the nodeId and connectionId got reversed, ca…
gregschohn ab3dfb4
Extra guards to try to make tests more reliable, but one of the FullT…
gregschohn 273c5aa
More test fixes, including fixing a regression that I had caused in a…
gregschohn e0167f5
Fix a race condition with commitKafkaKey.
gregschohn 3d81ad8
Two changes to kafka interactions. Add trace spans for traffic sourc…
gregschohn ae45a6a
Extract an IInstrumentationAttributes interface from IScopedInstrumen…
gregschohn 195d0ba
Checkpoint/WIP - More spans across the board, specifically through th…
gregschohn bef9944
Test bugfix. toString() wasn't threadsafe. Now it is.
gregschohn 8b50c89
Refactoring and code consolidation around context management.
gregschohn 0e5fe09
Refactoring. Which classes emit metrics, all scopes have names, and …
gregschohn 613a504
Refactor TestContext so that it doesn't use statics and allows caller…
gregschohn 7df0dcc
Remove a hardcoded path to my local directory
gregschohn de0d482
Fixed bugs in trace management and forced a lot more test code to tak…
gregschohn 72b1ca8
Add another scheduled span before the request is sent.
gregschohn 6043eed
Move all span names into virtual interface functions so that they can…
gregschohn 37b99eb
Pass more contexts, make contexts able to express more metrics, and e…
gregschohn 6684486
Minor bugfixes that make a huge difference. Fix a broken unit test a…
gregschohn 7bf4388
Some refactoring to increase the typesafety and to support greater co…
gregschohn 8ef0376
Fix mend security issue for json-path CVE by updating opensearch-secu…
gregschohn 37ae548
Remove zipkin as a tracing sink.
gregschohn 1172912
Make attribute name filtering more generic and fix a bug in negation …
gregschohn 22296b7
Minor tweaks to the otel collector (including renaming from 'demo') a…
gregschohn d1b237a
Set the aggregation temporality to delta rather than cumulative.
gregschohn fdd8141
Wrap all metric emissions within the context's span so that the metri…
gregschohn 490521d
In progress changes. I'm trying to track down a regression and want …
gregschohn f199e98
In-progress checkpoint (code won't compile). Setting up separate met…
gregschohn e110540
Another in-progress checkpoint (still won't compile) where I'm moving…
gregschohn 156ae72
Another checkpoint that still doesn't compile, but less files (I thin…
gregschohn 5dc32d9
Stop passing the Root telemetry scope as a generic parameter to all o…
gregschohn 320e9d8
Working on updating proxy code to get everything to compile.
gregschohn 9601a68
More refactoring, still doesn't all compile, but most of it does (top…
gregschohn 5f6bb3f
Fix the last of the compilation errors though tests are failing still.
gregschohn ccc0e2a
Bugfixes and test fixes to get all of the unit tests to pass.
gregschohn 0744424
Bugfixes. Stop metering double events in a couple spots and fix a co…
gregschohn 82f8ebb
Merge branch 'main' into ReplayerInstrumentation
gregschohn 35a9185
Upgrade otel libraries to 1.34.1 from 1.32 and add the enable_open_me…
gregschohn 0e8379d
Fix a bug where the current scope's attributes weren't being added in…
gregschohn a076bc3
Add a TestContext for every replayer test via inheritance on the Test…
gregschohn d3ee4f1
Change how MetricInstruments classes are instantiated.
gregschohn 54c5e27
Build fix - When refactoring to use TestContexts more globally, a tes…
gregschohn 0ce6694
Spin up a grafana container in the docker solution with simple creden…
gregschohn c8674eb
Start to get Source/Target comparison metrics in place and more refac…
gregschohn ffcc09b
Minor cleanup on exception tracking
gregschohn dd89336
Bugfix - a class was inheriting from the Connection context's MetricI…
gregschohn a15d08a
Cleanup build.gradle files' open-telemetry dependencies. Embrace ote…
gregschohn 5c454ca
Partial checkin to delete dead code and clean up imports and style is…
gregschohn bf8ea86
Addressing PR Feedback with some localized cleanups
gregschohn b641c5a
aws cli wasn't functional within my arm64 container because the docke…
gregschohn cd34b32
Rework otel-collector container packaging.
gregschohn a4173c0
PR feedback including:
gregschohn 2072f69
Change path from otelcol to otelCollector and enable the collector an…
gregschohn be689b5
Split the implementations of fillAttributes into two.
gregschohn 607ff05
Fix the dependencies for logging leaves and add 'processors' and 'rec…
gregschohn d480ed8
Setting the docker command for the otel-collector service to use the …
gregschohn 59db42f
Set the permissions for the otel container to write to cloudwatch and…
gregschohn 76c8c31
Minor cleanup
gregschohn 2cc67fd
Fix the runTestBenchmarks script to work when the endpoint uses http …
gregschohn 6e70d1a
Merge branch 'main' into ReplayerInstrumentation
gregschohn 9425ab6
Aesthetic formatting changes
gregschohn deb19a4
IInstrumentationAttributes no longer has scope related functionality.…
gregschohn a5f947e
Merge branch 'main' into ReplayerInstrumentation
gregschohn 9a31210
README documentation for the Instrumentation + some cleanup.
gregschohn 805e13b
When the first bucket size is <=0 for the CommonScopedMetricInstrumen…
gregschohn 9aa3432
Add tracing and metrics for replayer sockets as they're created and c…
gregschohn 48a45ab
Bugfix, test fix, lint fix.
gregschohn 16a9010
Fix an edge case where a socketChannel might not have been created ev…
gregschohn 4b102f7
Handle SocketContexts as first class contexts rather than trying to i…
gregschohn 1cb8927
Fix an issue with when to close the SocketContext and some memory lea…
gregschohn 441ca40
Tie off more loose ends for memory leaks during test runs.
gregschohn dc76239
Test fixes + make scheduled contexts use System.nanotime instead of I…
gregschohn 8a875b3
Set the x-ray exporter attribute index_all_attributes=true so that at…
gregschohn File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
15 changes: 15 additions & 0 deletions
15
...ensearch/migrations/trafficcapture/kafkaoffloader/tracing/IRootKafkaOffloaderContext.java
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,15 @@ | ||
package org.opensearch.migrations.trafficcapture.kafkaoffloader.tracing; | ||
|
||
import org.opensearch.migrations.tracing.IInstrumentConstructor; | ||
import org.opensearch.migrations.tracing.commoncontexts.IConnectionContext; | ||
|
||
public interface IRootKafkaOffloaderContext extends IInstrumentConstructor { | ||
KafkaRecordContext.MetricInstruments getKafkaOffloadingInstruments(); | ||
|
||
default KafkaRecordContext createKafkaRecordContext(IConnectionContext telemetryContext, | ||
String topicNameForTraffic, | ||
String recordId, | ||
int length) { | ||
return new KafkaRecordContext(this, telemetryContext, topicNameForTraffic, recordId, length); | ||
} | ||
} |
64 changes: 64 additions & 0 deletions
64
...a/org/opensearch/migrations/trafficcapture/kafkaoffloader/tracing/KafkaRecordContext.java
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,64 @@ | ||
package org.opensearch.migrations.trafficcapture.kafkaoffloader.tracing; | ||
|
||
import io.opentelemetry.api.common.AttributeKey; | ||
import io.opentelemetry.api.common.AttributesBuilder; | ||
import io.opentelemetry.api.metrics.LongCounter; | ||
import io.opentelemetry.api.metrics.Meter; | ||
import lombok.Getter; | ||
import lombok.NonNull; | ||
import org.opensearch.migrations.tracing.BaseNestedSpanContext; | ||
import org.opensearch.migrations.tracing.CommonScopedMetricInstruments; | ||
import org.opensearch.migrations.tracing.DirectNestedSpanContext; | ||
import org.opensearch.migrations.tracing.commoncontexts.IConnectionContext; | ||
import org.opensearch.migrations.tracing.IScopedInstrumentationAttributes; | ||
|
||
public class KafkaRecordContext extends | ||
BaseNestedSpanContext<IRootKafkaOffloaderContext, IConnectionContext> | ||
implements IScopedInstrumentationAttributes { | ||
public static final String ACTIVITY_NAME = "kafkaCommit"; | ||
|
||
static final AttributeKey<String> TOPIC_ATTR = AttributeKey.stringKey("topic"); | ||
static final AttributeKey<String> RECORD_ID_ATTR = AttributeKey.stringKey("recordId"); | ||
static final AttributeKey<Long> RECORD_SIZE_ATTR = AttributeKey.longKey("recordSize"); | ||
|
||
@Getter | ||
public final String topic; | ||
@Getter | ||
public final String recordId; | ||
@Getter | ||
public final int recordSize; | ||
|
||
public KafkaRecordContext(IRootKafkaOffloaderContext rootScope, IConnectionContext enclosingScope, | ||
String topic, String recordId, int recordSize) { | ||
super(rootScope, enclosingScope); | ||
this.topic = topic; | ||
this.recordId = recordId; | ||
this.recordSize = recordSize; | ||
initializeSpan(); | ||
} | ||
|
||
public static class MetricInstruments extends CommonScopedMetricInstruments { | ||
private MetricInstruments(Meter meter, String activityName) { | ||
super(meter, activityName); | ||
} | ||
} | ||
|
||
public static @NonNull MetricInstruments makeMetrics(Meter meter) { | ||
return new MetricInstruments(meter, ACTIVITY_NAME); | ||
} | ||
|
||
@Override | ||
public @NonNull MetricInstruments getMetrics() { | ||
return getRootInstrumentationScope().getKafkaOffloadingInstruments(); | ||
} | ||
|
||
@Override | ||
public String getActivityName() { return "stream_flush_called"; } | ||
|
||
@Override | ||
public AttributesBuilder fillAttributes(AttributesBuilder builder) { | ||
return builder.put(TOPIC_ATTR, getTopic()) | ||
.put(RECORD_ID_ATTR, getRecordId()) | ||
.put(RECORD_SIZE_ATTR, getRecordSize()); | ||
} | ||
} |
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
Oops, something went wrong.
Oops, something went wrong.
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.
Discussed offline, my preference is to chain our extending interface's fillAttributes so that our metrics are additive. May require adding an override to in between interfaces to satisfy java rules.
Also, call the attributeBuilder puts after the super call.
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.
This is cumbersome to do with Java, but the compiler does provide some protections. For a default method defined within a hierarchy of interfaces, I can specify the name of any interface after the deepest default implementation and those will compile without error. However, if refer to say
IInstrumentationAttributes.super.fillAttributes()
but then implement fillAttributes() in IWithStartTimeAndAttributes, the compiler will complain withjava: not an enclosing class: org.opensearch.migrations.tracing.IWithStartTimeAndAttributes