Skip to content

Commit

Permalink
Merge branch 'main' into fix-vectorTerm-segrep
Browse files Browse the repository at this point in the history
Signed-off-by: Rishikesh Pasham <[email protected]>
  • Loading branch information
Rishikesh1159 authored Aug 29, 2023
2 parents 4012c16 + bb7d23c commit 6f429b6
Show file tree
Hide file tree
Showing 36 changed files with 585 additions and 80 deletions.
6 changes: 5 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
- Bump `actions/setup-java` from 2 to 3 ([#9457](https://github.com/opensearch-project/OpenSearch/pull/9457))
- Bump `com.google.api:gax` from 2.27.0 to 2.32.0 ([#9300](https://github.com/opensearch-project/OpenSearch/pull/9300))
- Bump `netty` from 4.1.96.Final to 4.1.97.Final ([#9553](https://github.com/opensearch-project/OpenSearch/pull/9553))
- Bump `io.grpc:grpc-api` from 1.57.1 to 1.57.2 ([#9578](https://github.com/opensearch-project/OpenSearch/pull/9578))

### Changed
- Default to mmapfs within hybridfs ([#8508](https://github.com/opensearch-project/OpenSearch/pull/8508))
Expand Down Expand Up @@ -156,9 +157,11 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
- Improve performance of encoding composite keys in multi-term aggregations ([#9412](https://github.com/opensearch-project/OpenSearch/pull/9412))
- Fix sort related ITs for concurrent search ([#9177](https://github.com/opensearch-project/OpenSearch/pull/9466)
- Removing the vec file extension from INDEX_STORE_HYBRID_NIO_EXTENSIONS, to ensure the no performance degradation for vector search via Lucene Engine.([#9528](https://github.com/opensearch-project/OpenSearch/pull/9528)))
- Add support to use trace propagated from client ([#9506](https://github.com/opensearch-project/OpenSearch/pull/9506))
- Separate request-based and settings-based concurrent segment search controls and introduce AggregatorFactory method to determine concurrent search support ([#9469](https://github.com/opensearch-project/OpenSearch/pull/9469))
- [Remote Store] Rate limiter integration for remote store uploads and downloads([#9448](https://github.com/opensearch-project/OpenSearch/pull/9448/))
- [Remote Store] Implicitly use replication type SEGMENT for remote store clusters ([#9264](https://github.com/opensearch-project/OpenSearch/pull/9264))
- Use non-concurrent path for sort request on timeseries index and field([#9562](https://github.com/opensearch-project/OpenSearch/pull/9562))

### Deprecated

Expand All @@ -172,8 +175,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
- Handle null partSize in OnDemandBlockSnapshotIndexInput ([#9291](https://github.com/opensearch-project/OpenSearch/issues/9291))
- Fix condition to remove index create block ([#9437](https://github.com/opensearch-project/OpenSearch/pull/9437))
- Add support to clear archived index setting ([#9019](https://github.com/opensearch-project/OpenSearch/pull/9019))
- [Segment Replication] Fixed bug where replica shard temporarily serves stale data during an engine reset ([#9495](https://github.com/opensearch-project/OpenSearch/pull/9495))

### Security

[Unreleased 3.0]: https://github.com/opensearch-project/OpenSearch/compare/2.x...HEAD
[Unreleased 2.x]: https://github.com/opensearch-project/OpenSearch/compare/2.10...2.x
[Unreleased 2.x]: https://github.com/opensearch-project/OpenSearch/compare/2.10...2.x
24 changes: 24 additions & 0 deletions TESTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ OpenSearch uses [jUnit](https://junit.org/junit5/) for testing, it also uses ran
- [Iterating on packaging tests](#iterating-on-packaging-tests)
- [Testing backwards compatibility](#testing-backwards-compatibility)
- [BWC Testing against a specific remote/branch](#bwc-testing-against-a-specific-remotebranch)
- [BWC Testing with security](#bwc-testing-with-security)
- [Skip fetching latest](#skip-fetching-latest)
- [How to write good tests?](#how-to-write-good-tests)
- [Base classes for test cases](#base-classes-for-test-cases)
Expand Down Expand Up @@ -406,6 +407,29 @@ Example:

Say you need to make a change to `main` and have a BWC layer in `5.x`. You will need to: . Create a branch called `index_req_change` off your remote `${remote}`. This will contain your change. . Create a branch called `index_req_bwc_5.x` off `5.x`. This will contain your bwc layer. . Push both branches to your remote repository. . Run the tests with `./gradlew check -Dbwc.remote=${remote} -Dbwc.refspec.5.x=index_req_bwc_5.x`.

## BWC Testing with security

You may want to run BWC tests for a secure OpenSearch cluster. In order to do this, you will need to follow a few additional steps:

1. Clone the OpenSearch Security repository from https://github.com/opensearch-project/security.
2. Get both the old version of the Security plugin (the version you wish to come from) and the new version of the Security plugin (the version you wish to go to). This can be done either by fetching the maven artifact with a command like `wget https://repo1.maven.org/maven2/org/opensearch/plugin/opensearch-security/<TARGET_VERSION>.0/opensearch-security-<TARGET_VERSION>.0.zip` or by running `./gradlew assemble` from the base of the Security repository.
3. Move both of the Security artifacts into new directories at the path `/security/bwc-test/src/test/resources/<TARGET_VERSION>.0`. You should end up with two different directories in `/security/bwc-test/src/test/resources/`, one named the old version and one the new version.
4. Run the following command from the base of the Security repository:

```
./gradlew -p bwc-test clean bwcTestSuite \
-Dtests.security.manager=false \
-Dtests.opensearch.http.protocol=https \
-Dtests.opensearch.username=admin \
-Dtests.opensearch.password=admin \
-PcustomDistributionUrl="/OpenSearch/distribution/archives/linux-tar/build/distributions/opensearch-min-<TARGET_VERSION>-SNAPSHOT-linux-x64.tar.gz" \
-i
```

`-Dtests.security.manager=false` handles access issues when attempting to read the certificates from the file system.
`-Dtests.opensearch.http.protocol=https` tells the wait for cluster startup task to do the right thing.
`-PcustomDistributionUrl=...` uses a custom build of the distribution of OpenSearch. This is unnecessary when running against standard/unmodified OpenSearch core distributions.

### Skip fetching latest

For some BWC testing scenarios, you want to use the local clone of the repository without fetching latest. For these use cases, you can set the system property `tests.bwc.git_fetch_latest` to `false` and the BWC builds will skip fetching the latest from the remote.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@

import java.io.Closeable;
import java.io.IOException;
import java.util.List;
import java.util.Map;
import java.util.Optional;

/**
*
Expand Down Expand Up @@ -44,7 +47,7 @@ public SpanScope startSpan(String spanName) {

@Override
public SpanScope startSpan(String spanName, Attributes attributes) {
return startSpan(spanName, null, attributes);
return startSpan(spanName, (SpanContext) null, attributes);
}

@Override
Expand Down Expand Up @@ -97,4 +100,10 @@ protected void addDefaultAttributes(Span span) {
span.addAttribute(THREAD_NAME, Thread.currentThread().getName());
}

@Override
public SpanScope startSpan(String spanName, Map<String, List<String>> headers, Attributes attributes) {
Optional<Span> propagatedSpan = tracingTelemetry.getContextPropagator().extractFromHeaders(headers);
return startSpan(spanName, propagatedSpan.map(SpanContext::new).orElse(null), attributes);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
package org.opensearch.telemetry.tracing;

import org.opensearch.telemetry.tracing.attributes.Attributes;
import org.opensearch.telemetry.tracing.http.HttpTracer;

import java.io.Closeable;

Expand All @@ -18,7 +19,7 @@
*
* All methods on the Tracer object are multi-thread safe.
*/
public interface Tracer extends Closeable {
public interface Tracer extends HttpTracer, Closeable {

/**
* Starts the {@link Span} with given name
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@

package org.opensearch.telemetry.tracing;

import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.function.BiConsumer;

/**
Expand All @@ -23,7 +25,15 @@ public interface TracingContextPropagator {
* @param props properties
* @return current span
*/
Span extract(Map<String, String> props);
Optional<Span> extract(Map<String, String> props);

/**
* Extracts current span from HTTP headers.
*
* @param headers request headers to extract the context from
* @return current span
*/
Optional<Span> extractFromHeaders(Map<String, List<String>> headers);

/**
* Injects tracing context
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
/*
* SPDX-License-Identifier: Apache-2.0
*
* The OpenSearch Contributors require contributions made to
* this file be licensed under the Apache-2.0 license or a
* compatible open source license.
*/

package org.opensearch.telemetry.tracing.http;

import org.opensearch.telemetry.tracing.Span;
import org.opensearch.telemetry.tracing.SpanScope;
import org.opensearch.telemetry.tracing.attributes.Attributes;

import java.util.List;
import java.util.Map;

/**
* HttpTracer helps in creating a {@link Span} which reads the incoming tracing information
* from the HttpRequest header and propagate the span accordingly.
*
* All methods on the Tracer object are multi-thread safe.
*/
public interface HttpTracer {
/**
* Start the span with propagating the tracing info from the HttpRequest header.
*
* @param spanName span name.
* @param header http request header.
* @param attributes span attributes.
* @return scope of the span, must be closed with explicit close or with try-with-resource
*/
SpanScope startSpan(String spanName, Map<String, List<String>> header, Attributes attributes);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
/*
* SPDX-License-Identifier: Apache-2.0
*
* The OpenSearch Contributors require contributions made to
* this file be licensed under the Apache-2.0 license or a
* compatible open source license.
*/

/**
* Contains No-op implementations
*/
package org.opensearch.telemetry.tracing.http;
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@
import org.opensearch.telemetry.tracing.Tracer;
import org.opensearch.telemetry.tracing.attributes.Attributes;

import java.util.List;
import java.util.Map;

/**
* No-op implementation of Tracer
*
Expand Down Expand Up @@ -51,4 +54,9 @@ public SpanContext getCurrentSpan() {
public void close() {

}

@Override
public SpanScope startSpan(String spanName, Map<String, List<String>> header, Attributes attributes) {
return SpanScope.NO_OP;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@
import org.junit.Assert;

import java.io.IOException;
import java.util.Arrays;
import java.util.HashMap;
import java.util.List;
import java.util.Map;

import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.eq;
Expand Down Expand Up @@ -104,14 +108,36 @@ public void testCreateSpanWithParent() {
Assert.assertEquals(parentSpan.getSpan(), defaultTracer.getCurrentSpan().getSpan().getParentSpan());
}

public void testHttpTracer() {
String traceId = "trace_id";
String spanId = "span_id";
TracingTelemetry tracingTelemetry = new MockTracingTelemetry();

DefaultTracer defaultTracer = new DefaultTracer(
tracingTelemetry,
new ThreadContextBasedTracerContextStorage(new ThreadContext(Settings.EMPTY), tracingTelemetry)
);

Map<String, List<String>> requestHeaders = new HashMap<>();
requestHeaders.put("traceparent", Arrays.asList(traceId + "~" + spanId));

SpanScope spanScope = defaultTracer.startSpan("test_span", requestHeaders, Attributes.EMPTY);
SpanContext currentSpan = defaultTracer.getCurrentSpan();
assertNotNull(currentSpan);
assertEquals(traceId, currentSpan.getSpan().getTraceId());
assertEquals(traceId, currentSpan.getSpan().getParentSpan().getTraceId());
assertEquals(spanId, currentSpan.getSpan().getParentSpan().getSpanId());
spanScope.close();
}

public void testCreateSpanWithNullParent() {
TracingTelemetry tracingTelemetry = new MockTracingTelemetry();
DefaultTracer defaultTracer = new DefaultTracer(
tracingTelemetry,
new ThreadContextBasedTracerContextStorage(new ThreadContext(Settings.EMPTY), tracingTelemetry)
);

defaultTracer.startSpan("span_name", null, Attributes.EMPTY);
defaultTracer.startSpan("span_name");

Assert.assertEquals("span_name", defaultTracer.getCurrentSpan().getSpan().getSpanName());
Assert.assertEquals(null, defaultTracer.getCurrentSpan().getSpan().getParentSpan());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,16 +54,16 @@ public void testRunnableWithParent() throws Exception {
DefaultTracer defaultTracer = new DefaultTracer(new MockTracingTelemetry(), contextStorage);
defaultTracer.startSpan(parentSpanName);
SpanContext parentSpan = defaultTracer.getCurrentSpan();
AtomicReference<SpanContext> currrntSpan = new AtomicReference<>(new SpanContext(null));
AtomicReference<SpanContext> currentSpan = new AtomicReference<>();
final AtomicBoolean isRunnableCompleted = new AtomicBoolean(false);
TraceableRunnable traceableRunnable = new TraceableRunnable(defaultTracer, spanName, parentSpan, Attributes.EMPTY, () -> {
isRunnableCompleted.set(true);
currrntSpan.set(defaultTracer.getCurrentSpan());
currentSpan.set(defaultTracer.getCurrentSpan());
});
traceableRunnable.run();
assertTrue(isRunnableCompleted.get());
assertEquals(spanName, currrntSpan.get().getSpan().getSpanName());
assertEquals(parentSpan.getSpan(), currrntSpan.get().getSpan().getParentSpan());
assertEquals(spanName, currentSpan.get().getSpan().getSpanName());
assertEquals(parentSpan.getSpan(), currentSpan.get().getSpan().getParentSpan());
assertEquals(parentSpan.getSpan(), defaultTracer.getCurrentSpan().getSpan());
}
}
2 changes: 1 addition & 1 deletion plugins/repository-gcs/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ dependencies {
api "org.apache.logging.log4j:log4j-1.2-api:${versions.log4j}"
api "commons-codec:commons-codec:${versions.commonscodec}"
api 'org.threeten:threetenbp:1.4.4'
api 'io.grpc:grpc-api:1.57.1'
api 'io.grpc:grpc-api:1.57.2'
api 'io.opencensus:opencensus-api:0.31.1'
api 'io.opencensus:opencensus-contrib-http-util:0.31.1'

Expand Down
1 change: 0 additions & 1 deletion plugins/repository-gcs/licenses/grpc-api-1.57.1.jar.sha1

This file was deleted.

1 change: 1 addition & 0 deletions plugins/repository-gcs/licenses/grpc-api-1.57.2.jar.sha1
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
c71a006b81ddae7bc4b7cb1d2da78c1b173761f4
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,12 @@

package org.opensearch.telemetry.tracing;

import org.opensearch.core.common.Strings;

import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.function.BiConsumer;

import io.opentelemetry.api.OpenTelemetry;
Expand All @@ -32,15 +37,25 @@ public OTelTracingContextPropagator(OpenTelemetry openTelemetry) {
}

@Override
public Span extract(Map<String, String> props) {
public Optional<Span> extract(Map<String, String> props) {
Context context = openTelemetry.getPropagators().getTextMapPropagator().extract(Context.current(), props, TEXT_MAP_GETTER);
return Optional.ofNullable(getPropagatedSpan(context));
}

private static OTelPropagatedSpan getPropagatedSpan(Context context) {
if (context != null) {
io.opentelemetry.api.trace.Span span = io.opentelemetry.api.trace.Span.fromContext(context);
return new OTelPropagatedSpan(span);
}
return null;
}

@Override
public Optional<Span> extractFromHeaders(Map<String, List<String>> headers) {
Context context = openTelemetry.getPropagators().getTextMapPropagator().extract(Context.current(), headers, HEADER_TEXT_MAP_GETTER);
return Optional.ofNullable(getPropagatedSpan(context));
}

@Override
public void inject(Span currentSpan, BiConsumer<String, String> setter) {
openTelemetry.getPropagators().getTextMapPropagator().inject(context((OTelSpan) currentSpan), setter, TEXT_MAP_SETTER);
Expand Down Expand Up @@ -72,4 +87,23 @@ public String get(Map<String, String> headers, String key) {
}
};

private static final TextMapGetter<Map<String, List<String>>> HEADER_TEXT_MAP_GETTER = new TextMapGetter<>() {
@Override
public Iterable<String> keys(Map<String, List<String>> headers) {
if (headers != null) {
return headers.keySet();
} else {
return Collections.emptySet();
}
}

@Override
public String get(Map<String, List<String>> headers, String key) {
if (headers != null && headers.containsKey(key)) {
return Strings.collectionToCommaDelimitedString(headers.get(key));
}
return null;
}
};

}
Loading

0 comments on commit 6f429b6

Please sign in to comment.