Conversation
sean-
force-pushed
the
gh-800
branch
2 times, most recently
from
March 30, 2026 17:04
d17eef9 to
3ad158a
Compare
sean-
force-pushed
the
gh-816
branch
2 times, most recently
from
April 15, 2026 21:58
3479187 to
4fb9324
Compare
sean-
force-pushed
the
gh-800
branch
2 times, most recently
from
April 16, 2026 22:15
5472577 to
5e763cb
Compare
sean-
force-pushed
the
gh-800
branch
2 times, most recently
from
May 21, 2026 04:53
84c3cd3 to
34c3b88
Compare
…arch-project#801) * Skip shard routing tests on OpenSearch < 2.2.0 with security OpenSearch < 2.2.0 with the security plugin throws java.io.OptionalDataException on shard-routed requests due to non-thread-safe HashSet/HashMap in User serialization. Fixed in 2.2.0 by opensearch-project/security#1970 (50a94b47). Widen the existing 2.1.0-only skip to cover all versions below 2.2.0. Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * Add adaptive max_concurrent_shard_requests for search routing Derive per-request max_concurrent_shard_requests from a cluster-wide AIMD congestion window aggregated across all polled nodes' search thread pools. The value is clamped to configurable [min, max] (defaults 5-256) and only applied on non-shard-exact search routes through a coordinator. The cluster-wide signal (clusterSearchAIMD) sums delta(wait_time) and delta(completed) across all nodes each poll cycle, then runs a single AIMD state machine. This correctly models cluster-wide data-node capacity rather than a single coordinator's local thread pool pressure. Per-node AIMD for connection scoring remains unchanged. Falls back to per-node cwnd before the first poll cycle completes (clusterSearchCwnd=0 signals not-yet-ready). Caller-set query parameters are never overwritten. The feature is controllable via WithAdaptiveConcurrency(bool), WithAdaptiveConcurrencyLimits(min, max), OPENSEARCH_GO_SHARD_REQUESTS env var, or OPENSEARCH_GO_ROUTING_CONFIG=-adaptive_concurrency. Handle singleServerPool in pollNodeStats so AIMD updates proceed even when discovery collapses to a single connection (e.g. behind a proxy). Add nil-pool guard in fetchAndEvaluateNodeStats to skip overload demotion/promotion when there is no multi-server pool to demote within. Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * Refactor buildRouteEvent to use a params struct Replace the 16-parameter positional function signature with a routeEventParams struct. Zero-value fields are omitted at each call site, making the intent of each invocation clearer and eliminating a class of argument-ordering bugs. No functional change — purely mechanical refactor across all 7 call sites (poolRouter, indexRouter, docRouter, and test). Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * Set benchtime=200ms for reliable CI Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * Rename SkipIfSingleNode to RequireMinNodes; poll for node readiness SkipIfSingleNode did a single point-in-time /_nodes/http check. On slower clusters this raced with cluster formation, and on single-node CI (unreleased workflow) the polling fallback burned 60s before the Go test timeout killed everything. RequireMinNodes: - Reads OPENSEARCH_NODE_COUNT env var for instant skip when the cluster is known to be too small (no network calls needed) - Polls until the required nodes join when the cluster is expected to be large enough - Falls back to stability detection (3 consecutive identical counts) when the env var is unset Set OPENSEARCH_NODE_COUNT=3 in integration and compatibility CI workflows (docker-compose 3-node clusters) and =1 in the unreleased workflow (single bare process). Pass through Makefile to test runner. RequireMinNodes now polls until the required number of nodes have joined or a timeout expires, then skips if the cluster is permanently too small (e.g., single-node CI). Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * Rename "Pool Metrics" to "Policy Metrics" - PoolSnapshot -> PolicySnapshot (type, all methods, all references) - PoolReporter -> PolicyReporter (interface) - poolSnapshotCollector -> policySnapshotCollector (interface) - poolSnapshots() -> policySnapshots() (method name on all wrapper policies) - collectPoolSnapshots() -> collectPolicySnapshots() (function) - Metrics.Pools -> Metrics.Policies (field + JSON tag) Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * Refactor metrics collection from tree-walking to callbacks Replace forward tree-traversal in Metrics() with three callback types registered by policies at construction time: - ConnectionMetricCallback: batch per-connection augmentation - PolicyMetricCallback: per-policy pool snapshot - MetricsCallback: top-level Metrics augmentation (e.g., router cache) Leaf policies (RolePolicy, RoundRobinPolicy, CoordinatorPolicy) register PolicyMetricCallback; router policies (poolRouter, IndexRouter, DocRouter) register MetricsCallback for the router cache snapshot. Metrics() now deduplicates connections via map-set before building the snapshot, preventing double-reporting when a connection appears in multiple policy pools. Remove routerSnapshotProvider, collectRouterSnapshot, runMetricCallbacks, and TestCollectRouterSnapshot. Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * Remove metrics tree-walking infrastructure The callback-based metrics collection registered during configurePolicySettings() replaces the recursive tree-walking approach. Remove the now-unused interfaces, methods, and their tests: - PolicyReporter interface and compile-time checks - policySnapshotCollector interface and collectPolicySnapshots() - policySnapshots() from PolicyChain, MuxPolicy, IfEnabledPolicy, poolRouter - routerSnapshot() from poolRouter, IndexRouter, DocRouter policyTreeWalker and childPolicies() are retained - still used by shard placement updates, router cache lookup, and env override walks. Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * Add MCSR gauge to ConnectionMetric snapshots Register a ConnectionMetricCallback in the search pool router that reports each node's adaptive max_concurrent_shard_requests value. The callback is gated at registration by adaptiveConcurrencyEnabled(), so the per-snapshot path is unconditional — no locks needed since loadCwnd() is atomic and computeAdaptiveConcurrency() is pure math. Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * Two concurrent Next() calls could each spawn a deferredCapEnforcement goroutine. Between spawning and lock acquisition, Metrics() and discovery could read stale lifecycle bits, observing inconsistent active/standby counts. Replace go deferredCapEnforcement() with triggerCapEnforcement(), which acquires the write lock via TryLock before launching the goroutine. If the lock is held, enforcement is skipped and self-heals on the next Next() call. RLock callers (snapshot, Metrics) block until the goroutine releases the write lock, ensuring consistent observations. Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * refactor: pool score buffers and nil-guard connScoreSelect results Replace per-call [8]float64 stack buffers with a sync.Pool that ratchets to the working set size. Eliminates heap escapes for the common case while handling arbitrarily large candidate sets. Add nil checks on connScoreSelect return values — if all candidates are skipped (warmup, overload), return an empty NextHop instead of dereferencing nil. Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> --------- Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com>
…roject#836) Signed-off-by: shreyah963 <shreyab963@gmail.com>
…ct#830) When discovery temporarily sees a single node (rolling restart, network partition), demoteConnectionPoolWithLock replaces the multiServerPool with a singleServerPool. Resurrection goroutines from the old pool capture the pool pointer and loop on its context. Because the pool previously shared the Client's context directly, those goroutines never exit until the entire client is closed. Give each multiServerPool its own derived context. Cancel it on demotion so orphaned resurrection goroutines exit via <-ctx.Done(). Ref: opensearch-project#811 Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com>
…ct#834) * modernize: use WaitGroup.Go in two internal test files Apply Go 1.25 WaitGroup.Go(func) in connection_warmup_internal_test.go and selector_round_robin_internal_test.go, replacing the explicit go func() { defer wg.Done(); ... }() form. These were the two findings produced by `golangci-lint run --fix --build-tags "integration core plugins plugin_security plugin_index_management multinode"` on main. CI passes today because the workflow uses --fix, but the diagnostics are real and worth landing so contributors with a stricter local lint config see a clean tree. No behavior change. Signed-off-by: Sun Ro Lee <lsn3192@gmail.com> * chore: link PR opensearch-project#834 in changelog Signed-off-by: Sun Ro Lee <lsn3192@gmail.com> Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * modernize: convert remaining wg.Add(1)/go func patterns to WaitGroup.Go These sites were missed by the linter in CI because their `!integration` build tag excludes them from the integration-tagged lint run, and because the modernize analyzer skips goroutines that take arguments or aren't immediately preceded by wg.Add(1). They are the same idiom and convert cleanly under Go 1.25's per-iteration loop variables. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Sun Ro Lee <lsn3192@gmail.com> --------- Signed-off-by: Sun Ro Lee <lsn3192@gmail.com> Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
…pensearch-project#815) * Add configurable shard cost multipliers via env var and RouterOption Introduce OPENSEARCH_GO_SHARD_COST environment variable, WithShardCosts() RouterOption, and ShardCostConfig passthrough field to override shard cost multipliers at runtime for connection scoring tuning. Consolidate all shard cost types, constants, and tables into shard_cost_config.go for source code locality. Add ShardCostConfigError typed error for structured parse failure reporting. Extract server-side thread pool names into constants with a pointer to the authoritative ThreadPool.Names in the OpenSearch server source. Fixes: opensearch-project#814 Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * Add dynamic read cost scoring with write-pool-aware primary shedding Replace static primary/replica shard cost multipliers with a dynamic scoring function for read operations. When a read targets a primary shard, the effective cost scales with the node's write-pool utilization: effectiveCost = (1-primaryPct) * replicaCost + primaryPct * dynamicCost dynamicCost = base + amplify * (writeUtil ^ exponent) At idle, primaries are slightly preferred (cost 0.95 < replica's 1.0) for freshest data. As writes ramp up, reads progressively shed to replicas. For mixed nodes (common in production), primaryPct blends the cost proportionally based on the node's primary-to-total shard ratio rather than the previous all-or-nothing boolean. Key changes: - Add connScoreFunc type for pluggable per-operation scoring strategies - Add newReadScoreFunc() factory with configurable base/amplify/exponent - Replace abstract key config (preferred/alternate) with r:base/r:amplify/ r:exponent curve keys and concrete static keys - Rename calcConnScore -> calcConnDefaultScore (static fallback) - Log invalid OPENSEARCH_GO_SHARD_COST via debugLogger instead of silently swallowing - Update guides/routing.md with dynamic scoring docs, blending table, and rewritten env var configuration section Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * feat(transport): add pooledConns and pooledFloats buffer types Typed wrappers around sync.Pool for []float64 and []*Connection buffers with Slice(), Len(), and Release() methods. Zero-value is safe (Release is a no-op), and the types inline to the same machine code as raw pointer manipulation. These will be used by calcMultiKeyCost and connScoreSelect callers to eliminate per-request heap allocations on the multi-key routing hot path. Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * perf(transport): pool extraCost and scores allocations in routing Replace per-request heap allocations with sync.Pool buffers on the multi-key routing and score-selection hot paths: - calcMultiKeyCost now returns (pooledConns, pooledFloats) using pooled buffers for both the candidate list and extra-cost slice - All connScoreSelect call sites use acquireFloats/Release instead of the scoresBuf [8]float64 + overflow make pattern This eliminates 2-3 allocations per routed request when using multi-value routing keys (?routing=k1,k2,k3). Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * perf(transport): zero-alloc calcMultiKeyCost via slices.SortFunc Replace sort.Slice (which uses reflection and allocates 2-3 objects per call) with slices.SortFunc (generic, no reflection, no allocs). Combined with the pooledConns/pooledFloats buffers from the previous commit, the full multi-key routing path is now zero-allocation: BenchmarkCalcMultiKeyCost/keys=2 250 ns/op 0 B/op 0 allocs/op BenchmarkCalcMultiKeyCost/keys=5 464 ns/op 0 B/op 0 allocs/op BenchmarkCalcMultiKeyCost/keys=10 848 ns/op 0 B/op 0 allocs/op BenchmarkConnScoreSelect/8_cands 163 ns/op 0 B/op 0 allocs/op Also adds shard_routing_benchmark_test.go with benchmarks for calcMultiKeyCost, calcSingleKeyCost, and connScoreSelect. Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * perf(transport): replace sort.Slice with slices.SortFunc in pool dead-list sort.Slice uses reflection and allocates 2–3 objects per call; slices.SortFunc is generic and zero-alloc. Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * perf(transport): zero-alloc calcSingleKeyCost via pooledConns and pooledNodeSet Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * style(transport): fix goconst and modernize lint issues Extract repeated string literals into package-level constants: - Policy type names (policyTypeNameChain, policyTypeNameRouter, etc.) - Cluster health status values (clusterStatusGreen/Yellow/Red) - Connection lifecycle state names (lcNameReady, lcNameActive, etc.) - Shard cost config error reasons and key names - Pool name references (use existing poolSearch constant) - Test util version expression (versionAllSupported) Auto-applied modernize fixes: - sync.WaitGroup Add/go/Done -> wg.Go() pattern - Manual backward loop -> slices.Backward() Triggered by golangci-lint 2.12.2 goconst update detecting repeated string literals across production and test code. Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * feat(transport): add OPENSEARCH_GO_ROUTER env var to enable DefaultRouter Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * feat: change DiscoverNodesOnStart bool→*bool with env var inheritance Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * chore(transport): clarify psIsEnabled doc and apply goimports drift Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * refactor: consolidate OPENSEARCH_GO_* env var names in internal/envvars Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * ci: retrigger flaky integ tests Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> --------- Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com>
Signed-off-by: Divya Madala <divyaasm@amazon.com>
…h-project#804) * Fix alias/mapping/settings/block API URL path when Indices is empty (opensearch-project#650) When Indices is empty, path construction produced a double-slash (e.g. "//_alias/myalias") which http.NewRequest misparses per RFC 3986, treating "_alias" as the URL authority. After setReqURL overwrites the host, the path loses the API prefix entirely, causing requests to hit the wrong endpoint (e.g. creating an index instead of an alias). Extract buildPath helper that skips empty segments, eliminating the class of bug and the magic-number Grow hints. Benchmarked at 1 alloc / ~21ns vs path.Join's 3 allocs / ~63ns. Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * Fix double-slash URL path bug in 74 GetRequest methods (opensearch-project#650) When optional path segments (Indices, Index, Repo, etc.) are empty, path construction produced "//" which http.NewRequest misparses per RFC 3986, treating the next segment as the URL authority. After the transport overwrites the host, the path loses its API prefix, causing requests to hit the wrong endpoint. Extract opensearch.BuildPath helper that skips empty segments (1 alloc, ~21ns - benchmarked against path.Join's 3 allocs, ~63ns; see BenchmarkBuildPath in opensearchapi/path_test.go). Convert all 74 vulnerable GetRequest methods across opensearchapi, plugins/security, and plugins/ism to use it. Fixes: opensearch-project#617, opensearch-project#650 Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * Add typed path segment types as a foundation for URL construction safety Introduce 21 domain-specific string types (Index, Indices, Action, DocumentID, Alias, Repo, Snapshot, NodeID, Plugin, Policy, Block, Prefix, Suffix, Name, Resource, Attr, Value, Metric, IndexMetric, Metrics, NodeFilter) and 25 struct-per-shape path builders. Build() methods skip empty optional segments, eliminating the double-slash URL bug (opensearch-project#617, opensearch-project#650) at the type level. Indices filters empty-string elements during comma-join so mixed slices like {"", "x", ""} produce "/x" rather than "/,x,"; required-slice fields are validated via hasNonEmpty() to reject all-empty inputs. Segment encoding (url.PathEscape) is layered on by subsequent commits in this series; the typed shapes make that layering possible. Published API types in opensearchapi/ and plugins/ remain string/ []string. Casts to domain types happen internally in GetRequest() methods, preserving backward compatibility. Fixes opensearch-project#617, Fixes opensearch-project#650 Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * Add Docker image pre-pull with retry for CI resilience Add retry loop (30 attempts, 10s backoff) to pre-pull the OpenSearch base image before docker compose build. Handles transient Docker Hub 504 timeouts and BuildKit gRPC disconnects (as seen in CI failures in opensearch-project#804). Applied in the Makefile cluster.docker-build target so both CI and local invocations get the same protection. Makefile reliability improvements (exit-on-final-failure guard, and dropping the redundant workflow-level pre-pull step now that the Makefile target handles it) folded in from opensearch-project#833 review feedback by @Jakob3xD and @ssunno. Fixes: opensearch-project#833 Co-authored-by: Jakob <jakob.hahn@hetzner.com> Co-authored-by: ssunno <ssunno@users.noreply.github.com> Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * Pool path builder buffers to eliminate per-request allocation churn Replace strings.Builder in all 25 typed path Build() methods with a sync.Pool'd []byte buffer. Each Build() acquires a pooled buffer, writes path segments via append, copies to a final string on release, and returns the buffer for reuse. Under steady state the working buffers are recycled so there is no allocation for the buffer itself. Because append handles growth, the entire pre-compute length calculation phase is no longer needed. This removes reqSegLen, optSegLen, reqSegWrite, optSegWrite, Indices.joinLen, Indices.join, Indices.optSegLen, and Indices.optSegWrite. Buffers exceeding 4 KiB are discarded rather than returned to the pool to bound memory growth from pathological inputs. Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * Add spec-driven path builder generator with pooled zero-alloc construction Generate type-safe path builder structs from the OpenAPI specification. Each builder validates required fields at build time and uses a sync.Pool-backed byte buffer to construct URL paths with zero per-request heap allocations. Includes benchmark tests covering single-index, multi-index, multi-segment, and static-path patterns. Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * Migrate all consumer files to typed path builders Replace hand-built URL paths in 146 API consumer files with generated ospath.*Path{} builders, gaining compile-time segment validation and eliminating fmt.Sprintf allocation at call sites. The typed builders accept []string for multi-value segments directly, removing the need for callers to pre-join with strings.Join. Add internal/build for [http.Request] construction without the url.Parse overhead of [http.NewRequest]. RawPath + Path are set together so url.URL.EscapedPath() honors the percent-encoded form produced by the path builders. build.Request: - Skip the default Content-Type seed when the caller supplies one, so callers that need application/x-ndjson (e.g. _bulk, _msearch) don't end up with two Content-Type values on the wire. - Reject paths with invalid percent-encoding (e.g. "%ZZ"). Falling through to "Path = raw" plus "RawPath = raw" makes EscapedPath() silently re-encode the literal '%' as "%25", corrupting the wire. - Tighten validMethod to RFC 7230 token chars only ("get", "GET,POST", "PROP[]FIND" are now rejected, matching net/http's isTokenTable). Export HeaderContentType, ContentTypeJSON, and ContentTypeNDJSON constants so generated client code can reference them by name. Remove the monolithic root-level path.go that predated the generated builders. Rewrite api_indices_response_test.go as table-driven tests. Add api_path_integration_test.go covering GetRequest() path correctness for representative API types. Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * Add api subcommand and restructure cmd/osgen for paths/api split Reorganize cmd/osgen so a single binary generates both typed path builders (paths subcommand) and API consumer structs (api subcommand) from the OpenAPI spec. Rename analyze.go/group.go/testgen.go to paths_analyze.go/paths_group.go/paths_testgen.go to make room for the api_*.go counterparts. Add shared naming.go for consistent acronym expansion (ID, UUID, HTTP, SQL, etc.) across both subcommands. Add internal/apiutil with FormatDuration and the shared Inspect type used by generated API consumer code. Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * Add emit/ template fragments and ir/ IR layer to osgen Restructure the API code generator with two new sub-packages: - cmd/osgen/emit/ holds template fragments for request structs, test scaffolding, plugin dispatch, and shared build code (~2400 lines split across frag_req.go, frag_tests.go, frag_plugin.go, frag_dispatch.go, build.go). - cmd/osgen/ir/ defines the intermediate representation that the api subcommand walks; cmd/osgen/ir_bridge.go converts the parsed OpenAPI spec into IR. Key behaviors: - Deterministic output: ops sorted by URL then operationId, with the spec's declared primary preserved as a stable tiebreaker. - Stale _gen.go files not produced by the current run are removed. - Output directories are validated against the git working tree root via os.OpenRoot to prevent accidental writes outside the repository. Drop internal/path/builders_gen.go; the next commit regenerates it through the unified osgen pipeline. Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * Add integration test generator and per-plugin client generation to osgen Extend the osgen api subcommand to produce integration tests for all 423 operations and per-plugin Client types with typed dispatch methods. - api_testgen_integ.go: template and classification logic for integration tests with fixture management, version skipping, and NeedImport* flags - api_render_plugin_client.go: per-plugin Client struct, generic do[T], dispatch methods, and internal/test helpers (NewClient, CreateFailingClient) - api_cmd.go: plugin detection fix (opensearchAPIPkgName vs corePkg), generation loop for plugin clients and test helpers - route.go: modulePath const, _common core group, methodNameFromSuffix - Hand-written test renames to TestManual_ prefix to avoid conflicts - VerifyInspect: require instead of assert, nil-safe Response check - testutil: fix context cancel leak in WaitForClusterReady Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * Generate osapi integration tests and plugin clients from spec Output of: cmd/osgen api -spec opensearch-openapi.yaml -out osapi -pkg osapi -plugins-out osapi/plugins - 167 core integration tests (osapi/*_integ_gen_test.go) - 256 plugin integration tests (osapi/plugins/*/*_integ_gen_test.go) - 21 plugin client types with dispatch methods (client_gen.go) - 21 plugin test helpers (internal/test/helpers_gen.go) - Compat, types, unions, unit tests, and params for all 423 operations Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * Use http.Status constants in production code Replace magic-number HTTP status codes with http.Status* constants in opensearchtransport/logger.go and opensearchtransport/opensearchtransport.go. The logger range checks tighten from "0-299" to "100-299" (status codes under 100 are invalid HTTP and were previously colored as success). opensearchutil/bulk_indexer.go also widens the per-item success range from {200,201} to 200-299 to match HTTP success semantics. OpenSearch bulk per-item responses are 200 or 201 in practice, so this is a no-op on the live response surface; the change closes a latent gap if a future server returns 202-299 for an accepted-but-still-in-progress item. Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * Use http.Status constants in test code Replace magic-number HTTP status codes with http.Status* constants in unit and integration tests across opensearchapi, opensearchtransport, and opensearchutil. Mirrors the same substitution applied to production code in the prior commit. Six files touched, no behavior change: every literal maps 1:1 to its named constant (502 -> http.StatusBadGateway, 404 -> http.StatusNotFound, etc.). Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * Use http.Method and http.Status constants in samples Replace HTTP method and status-code literals in _samples/ with their http.Method* and http.Status* constants: - _samples/json.go: "GET"/"PUT"/"POST"/"DELETE" -> http.MethodGet/Put/ Post/Delete in http.NewRequest calls. - _samples/bulk.go: `resp.Status > 299` -> `resp.Status >= http.StatusMultipleChoices` (semantically identical for integers). No behavior change. The samples are reference material for end users, so the named-constant form is more instructive than literals. Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * Add patched OpenSearch OpenAPI specification for code generation Vendor the bundled OpenAPI spec from opensearch-api-specification (x-api-version 2.16.0) as the input for cmd/osgen. Five upstream-pending fixes are applied locally and removed from the patch set as each lands upstream: - Fix ISM index parameters to use Indices type instead of IndexName - opensearch-project/opensearch-api-specification#1098 - Fix ltr CacheStatsResponse to use NodesResponseBase via allOf - opensearch-project/opensearch-api-specification#1099 - Add missing properties to nodes.info settings schemas - opensearch-project/opensearch-api-specification#1101 - Mark create_pit keep_alive query parameter as required - opensearch-project/opensearch-api-specification#1102 - Add max_score to HitsMetadata required fields - opensearch-project/opensearch-api-specification#1103 A sixth fix (#1107, CatResponseFormat schema unification) lands in the follow-up regen commit. Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * Add union types, version filtering, and field constants to osgen Extend cmd/osgen with three substantial features: - Union types: api_render_union.go and resp_union.go generate Go union-shaped types from anyOf/oneOf schemas with disambiguating branch identifiers. - Version filtering: cmd/osgen/version_filter.go applies the bundled spec's x-version-added / x-version-removed extensions to emit only operations valid for the targeted OpenSearch version. - Path field constants: fieldIndex, fieldID, fieldMetric, etc. replace magic strings throughout the emitter. Regenerate internal/path/builders_gen.go and builders_gen_test.go; this is where percent-encoding (encSeg) and hasNonEmpty validation land in the generated path builders. Add internal/params/params.go for shared query parameter helpers used by generated client code. Refinements: - unionConstName always concatenates unionName + branchName + "Type" (no de-stutter): a de-stutter shortcut would let two branches in the same union ("Bar" and "FooBar" under union "Foo") collide on "FooBarType". Uniqueness wins over elegance. - mapValueTypeName recurses through nested map / slice / pointer prefixes so types like "map[string]map[string]X" or "[]*X" resolve to a valid base identifier instead of feeding "*X" or "map[string]X" into baseGoName. - deriveBranchName runs the goTypeName fallback through baseGoName so dotted package qualifiers don't leak as identifier fragments. - Exclusion guards an empty versionAdded so the breadcrumb message doesn't render "requires OpenSearch >= " (empty version). - Drop the unused (and inverted) MinSatisfied helper. Propagate the spec's IsNDJSON flag through the IR (ir.Operation, ir_bridge.go) into the Req template so generated GetRequest methods inject Content-Type: application/x-ndjson before calling build.Request for _bulk, _msearch, and _msearch/template. Caller-provided Content-Type still wins. Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * Document osapi rename in cmd/osgen/README.md Update the api subcommand documentation so it matches what `make gen` actually invokes: - `-out ../../opensearchapi` -> `-out ../../osapi` in both example invocations (full regen and single-operation). - `-plugins-out ../../plugins` -> `-plugins-out ../../osapi/plugins`. - Add `-pkg` to the api subcommand flag table; was previously documented for paths but absent here, so a reader following the README would generate into the wrong directory under the wrong package name. Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * Document osapi v5-track surface in DEVELOPER_GUIDE.md Two changes to the Code Generation section: - Update the output-path paragraph from `opensearchapi/`/`plugins/` to the actual destinations of `make gen` at this point in the branch: `osapi/` and `osapi/plugins/`. (A later commit relocates these under `v5preview/opensearchapi/`; the guide is updated again at that step.) - Add a paragraph identifying `osapi/` as the v5-track API surface that coexists with hand-written `opensearchapi/` during the v4 -> v5 transition, with pointers to `osapi/README.md` and `UPGRADING.md` for usage and migration guidance. Drive-by: trailing-whitespace realignment of the GEN_* variable table (cosmetic, no content change). Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * Sync osgen flag help, godoc, and test fixtures with osapi/ output Mechanical rename of "opensearchapi" -> "osapi" in places where the generator's own metadata had drifted from the actual output package. No behavior change. - cmd/osgen/api_cmd.go: -out and -plugins-out flag help text now show the osapi/ paths; the usage error string updates likewise. - cmd/osgen/README.md: routing description points at osapi. - cmd/osgen/naming.go: pkgScopedName godoc now references the osapi package by name. Cosmetic gofmt re-alignment of scalarAliases map literals. - cmd/osgen/naming_test.go: golden expectations switch to osapi/*_gen.go paths; godoc reflects osapi/ and osapi/plugins/. - cmd/osgen/cmd_test.go: test calls pass "osapi" explicitly instead of the opensearchAPIPkgName const, and the stale-file fixture writes `package osapi` instead of `package opensearchapi`. Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * Regenerate osapi from osgen with applied spec patches Output of `make gen` after the union types, version filtering, and field constants added to cmd/osgen in the previous commit. The bundled spec at this point includes six upstream-pending fixes: - opensearch-project/opensearch-api-specification#1098 - opensearch-project/opensearch-api-specification#1099 - opensearch-project/opensearch-api-specification#1101 - opensearch-project/opensearch-api-specification#1102 - opensearch-project/opensearch-api-specification#1103 - opensearch-project/opensearch-api-specification#1107 The regenerated tree covers ~480 typed API operations across osapi/ and osapi/plugins/ with happy-path integration test scaffolding. Drive-by: a one-line trim in cmd/osgen/paths_render.go to keep the generator's output stable, an osapi/README.md describing the package layout, and an osapi/bench_getrequest_test.go for path-construction benchmarks. Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * Default integration cluster to SECURE_INTEGRATION=true The secure configuration (TLS + basic auth) is the production-realistic default. Callers who need insecure testing explicitly set SECURE_INTEGRATION=false. Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * Add container runtime detection and test output capture to Makefile Detect nerdctl or docker at build time (overridable via CONTAINER_RUNTIME). Pipe test output through tee to test-unit.log and test-integ.log for post-failure inspection while preserving exit codes via PIPESTATUS. Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * Pin CONTAINER_RUNTIME=docker in CI workflows GitHub Actions runners use docker, not nerdctl. Explicitly set the variable so the new Makefile runtime detection selects the correct backend. Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * Add CI workflow to verify generated code is up to date Runs make gen and surfaces a diff of the working tree afterward, catching cases where someone modifies the spec or generator without regenerating. The job is marked continue-on-error to surface staleness as a warning without blocking PRs while the generator stabilizes; flip to blocking once the regen is reliably hermetic. Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * Bump golangci-lint to 2.12.2 - Rename gomodguard to gomodguard_v2 in golangci-lint config. - Resolve new warnings introduced by the bump: extract lifecycle state names to lcName* constants in connection_lifecycle.go; similar magic-string extraction in policy_override_env.go and related files. Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * Modernize opensearchtransport with Go 1.25 idioms Use slices.Backward for reverse iteration in pool dead-list removal. Replace manual wg.Add/go/defer patterns with sync.WaitGroup.Go in tests. Drop a now-unneeded //nolint:nestif annotation. Drive-bys discovered while exercising the modernized tests: - Add nil-Header guards in setReqUserAgent and setReqGlobalHeader so synthetic *http.Request values (Header == nil) no longer panic. - Switch t.Context() to context.Background() inside t.Cleanup so the cleanup request isn't canceled by test-end before completing. - Split the api_tasks_test.go cleanup so a require-failure Goexit in the task-cancel cleanup doesn't skip the index-delete cleanup. - Skip TestTasksClient on OpenSearch <2.18 (.tasks index lacked cancellation_time_millis and resource_stats fields, OpenSearch#16201). Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * Extract shared integration helpers and add version guards to shard routing tests - Add opensearchtransport/integration_test_helpers_test.go for helpers shared across the shard routing integration tests (selector readiness predicates, connection-count requirements, etc.). - Expand opensearchtransport/testutil/helpers.go: ConnPollOpts + DefaultConnPollOpts + RequireMinConns for parameterized polling, and thread an explicit context.Context through RequireMinNodes (callers pass t.Context() or another scope-appropriate ctx). - Skip shard routing tests on OpenSearch <2.0 where routing_num_shards is unavailable from _cluster/state/metadata. - Replace magic 500 with http.StatusInternalServerError throughout the shard routing tests. Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * Add TestFlag bitfield framework for integration test behavior Replace the ad-hoc TestVersionOverrides map and integSkipReason() function with declarative TestRule records (glob NamePattern, Version constraint, TestFlag bitfield). The bitfield form lets a single rule both skip on old versions and inject a cluster-readiness poll, avoiding the previous duplication and making it cheaper to add new test-time behaviors (timeouts, fixture setup, etc.) without touching the emitter. Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * Regenerate integration tests with TestFlag framework Output of `make gen` after the prior commit added the TestFlag bitfield framework. The framework's TestRule table emits two kinds of additions into existing _integ_gen_test.go files: - WaitForAllNodesReady prologue for tests sensitive to lagging node registration: cat-nodes, nodes-info, nodes-stats. - t.Skip directives with bug links for tests that depend on fixtures the integration cluster doesn't provide: - delete_all_pits - malformed error body when no PITs exist (OpenSearch#11711) - field_caps - requires an index with mapped fields - get_script_languages - KNN plugin NPE in getSupportedContexts (k-NN#560) Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * Add Response.RawBody for buffered body access and modernize error tests Response gains a rawBody field, RawBody() accessor, HijackBody() ownership-transfer accessor, and NewResponse() constructor so generated response types can buffer and re-expose the raw bytes without consuming the stream. RawBody returns a pointer-stable view of the buffer for inspection; HijackBody clears r.rawBody and transfers ownership to the caller for log/background-process scenarios that outlive the Response. error_test.go is rewritten: - switch from assert to require so a broken expectation halts the subtest before subsequent dereferences panic - consolidate around table-driven cases for the multi-shape error bodies (StructError, StringError, error.cause chains, etc.) Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * Add unified security guide and eliminate raw JSON injection patterns Add guides/security.md covering TLS, credential management, path-param validation, request body construction, error-handling/info-disclosure, and transport configuration. Replace fmt.Sprintf-based JSON construction in guides/search.md and guides/tasks.md with opensearchutil.NewJSONReader so the examples demonstrate the safe pattern. Add cross-reference notes to the other existing guides pointing at security.md. Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * Harden link checker with retries and exclude flaky badge URL Add --max-retries=3 --retry-wait-time=10 to the lychee args so transient 504s from upstream don't fail the run, and exclude the codecov badge URL which intermittently 502s. Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * Add cmd/osgen coverage upload to unit CI workflow Bump Go CI timeouts to exceed the internal FSM timeouts (~6.5min) Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * Add golden workflow integration tests for core osapi operations Hand-written table-driven tests covering Bulk, Count, Cat, Alias, Settings, Mapping, Scroll, IndexTemplate, PIT, Aggregation, DocumentGet, Mget, Update, and DeleteByQuery. Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * Add per-directory codecov coverage targets The osapi/ package contains ~194k lines of generated code covering every API in the OpenSearch spec. Coverage sits at ~25% because many generated APIs target plugin-specific endpoints (ISM, anomaly detection, k-NN, security, notifications) that require plugins not present in the CI cluster. Unit-level roundtrip tests exercise request construction and dispatch for all 167 APIs, but full response parsing coverage requires integration tests against a cluster with the relevant plugins enabled. Expanding the CI cluster configuration and adding plugin-aware integration tests would raise this target over time. Component-level targets: core (transport, utils, signer): 80% - mature, hand-written code internal/path: 95% - small, fully testable osapi: 25% - limited by CI plugin set opensearchapi (legacy): 14% - frozen, being replaced Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * Use ospath builders instead of fmt.Sprintf for URL construction in tests Replace all fmt.Sprintf URL path construction with typed ospath builders (IndicesCreatePath, IndicesDeletePath, SearchPath, SearchShardsPath, IndexPath, ClusterHealthPath, ClusterStatePath) for compile-time safety and proper percent-encoding of path segments. Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * Add internal/test/readiness package for layered FSM-based test gating Each per-node FSM packs an ordinal Layer (lower 16 bits, cumulative) and a free-form ClientState (upper 16 bits) into one atomic uint32 so the satisfaction check collapses to s & target == target. Layers gate each other (no LayerHTTP without LayerTCP); ClientState bits flap freely. Per-layer budgets are tuned for CI pessimism. NodeFSM.Advance captures the caller's intent at the first state observation: a forward call (next > initialLayer) that loses the CAS to a peer that already advanced past next is a silent no-op, not a regression. Without this, two concurrent forward Advance calls would mis-classify the loser's retry against a now-higher layer as a deliberate regression and trip MaxRegressions. Cluster.lastPolledAt sits inside c.mu alongside lastErr/lastResp/polls with SetLastPolledAt/LastPolledAt accessors so the diagnostic dump reads a consistent snapshot. NodeFSM.Snapshot returns state, enteredAt, regressions, and history under the FSM's mu so renderNode emits a coherent per-node line. The old four independent loads could skew across a concurrent transition mid-render. The misconfig guard fails fast on any non-zero target without a LayerCheck or FSMCheck (was: only layer-bearing targets), so a state-only target with no driver no longer silently times out. Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * Replace WaitForAllNodesReady inline polling with readiness.Wait The previous implementation polled cat-nodes for 60s with a single boolean predicate, producing "Condition never satisfied" on timeout. Switch to readiness.Wait(t, ctx, TargetClusterReady, WithCluster(c)) which observes per-node progression through LayerHTTP, LayerClusterJoin, and LayerStatsReady, and dumps a structured per-node diagnostic with the full last cat-nodes response on failure. Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * Preserve in-flight shards in shardMap across /_cat/shards relocation window /_cat/shards parsing skips RELOCATING and INITIALIZING rows, so a shard mid-relocation is absent from placement.ShardToNodes for the duration of the relocation window. The previous unconditional store would clobber a complete cached map with this partial response, leaving Shards[N] == nil for the in-flight shard and disabling shard-exact routing for it. Carry forward entries the new response is missing: a shard in RELOCATING state still serves reads from the source until the destination flips to STARTED, so the prior entry remains a valid routing target for the relocation window. New data wins where present. Use CompareAndSwap with a bounded retry to bound the visible Load+merge+Store window: a CAS loss can drop a strictly fresher in-flight observation, so cheap retries (3 attempts) are preferred over a single attempt with silent loss. Log on retry exhaustion to surface persistent contention from a hot health-check goroutine; the next merge cycle will self-heal. Compute NumberOfPrimaryShards from the cross-state primary set (counting distinct shard numbers across STARTED + RELOCATING + INITIALIZING rows) rather than from ShardToNodes alone. ShardToNodes is filtered to STARTED entries, so a relocating shard would otherwise undercount the primary count and cause the post-merge prune to drop the very entry that carry-forward just preserved. The prune now fires only when the placement covers every primary shard (a real shrink), never on a partial view. Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * Mark singleServerPool's connection lcActive at construction Route all four singleServerPool construction sites through a newSingleServerPool helper that CAS's lcActive (clearing lcUnknown|lcStandby) under conn.mu; without it the no-op OnSuccess/OnFailure leave the connection lcUnknown for the lifetime of the pool, so observers classify the only available connection as not-ready. Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * Set lcReady when warmup completes guides/routing.md documents the post-warmup state as lcReady|lcActive but tryWarmupSkip's completion path only cleared lcNeedsWarmup, leaving lcReady unset for the lifetime of the connection. Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * Bump AWS SDK deps config v1.32.17 -> v1.32.18 credentials v1.19.16 -> v1.19.17 ssooidc v1.35.21 -> v1.36.0 Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * Mark ToPointer() in advance of v5 Add a package-local unexported ptr() helper to the opensearch and opensearchapi packages and route ToPointer through it. ToPointer is flagged Deprecated; the helper is removed in v5. Once the module's go directive moves to 1.26, ptr() itself can be deleted and call sites can switch to the native new(value) form (e.g. new(false)). Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * Parallelize osgen generated output - Target rendering uses runtime.NumCPU() goroutines with sorted output for determinism. - Added sync.WaitGroup.Go() workers in api_cmd renderloop - Split Makefile gen into gen-paths and gen-api targets for make -j gen support Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * Build bool query params as *bool - All 352 boolean query parameters now emit as *bool, enabling callers to send both true and false values. Fixes inability to override server-default-true params (wait_for_completion, flush, etc). - classifyParamSchema returns "*bool" instead of "bool" for booleans - Encoder: if r.X != nil { set("name", strconv.FormatBool(*r.X)) } - All test expectations updated Fixes: opensearch-project#840 Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * Lint cmd/osgen as a separate Go module Add a `cmd/osgen/` exclusion to .golangci.yml's goconst rule (the generator uses many short repeated literal strings as identifiers and import paths inside template input that don't benefit from const extraction). Extend the `lint.local` Makefile target to run golangci-lint inside cmd/osgen too, since it's a separate module not covered by the root invocation. Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * Tighten path builder generator: visibility, determinism, error paths - F18: emit opExplainCheck for case=1 + positional deps, closing a silent-fall-through path the explainXxx helper was meant to catch. - F24: unexport the path.Builder interface; the assertions are intra-package and no external caller writes path.Builder. - F25: drop the unused paramSet accumulator in pathParamInfo. - F26: iterate spec.Paths and per-path Operations() in sorted order so description, versionAdded, and docsURL captured for multi-method URLs are deterministic instead of depending on map iteration order. - F27: panic on unknown HTTP methods in httpMethodConst instead of silently falling through to MethodGet, so a spec typo or new verb fails the generator loudly. - F36: clarify alwaysImplies vacuous-case behavior; the function intentionally returns false when no path contains the dependent. Regenerated internal/path/builders_gen.go and builders_gen_test.go. Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * Generate per-required-field error cases and *bool false-case in path tests Improve generator-emitted test coverage: - Path test emit (cmd/osgen/paths_testgen.go): for builders with 2+ required fields, emit a "missing <Field>" case per required field with the others set; for builders with list fields, emit a multi-value case for each list field, not just the first; for builders with only optional fields, emit "only <FieldName>" cases per field. Replace require.Error with require.ErrorIs against errRequired so the public error contract is verified, not just presence. - Params test emit (cmd/osgen/emit/build.go, frag_tests.go): emit two scenarios for each *bool query param ("=true" and "=false") so the wire-level encoding of an explicit false is exercised. Regenerate internal/path/builders_gen_test.go and the osapi/*_internal_gen_test.go family. Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * Strengthen assertPathRoundTrip URL invariant checks Replace the per-segment self-consistency round-trip with a stricter helper that catches structural bugs the encoder is meant to prevent: - url.ParseRequestURI must succeed (valid request-URI shape). - Path must not contain "//" (the original opensearch-project#650 double-slash bug). - Decoded segments must not still decode further (no double-encoding). - Decoded segments must not contain "/", "?", "#" (path injection). - Stable encode-decode round-trip on every sub-segment. Used by every generated builder test. Catches encoder regressions that the previous self-consistency check would have silently accepted. Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * Run golangci-lint --fix on cmd/osgen and remove dead render pipeline - Run golangci-lint --fix across cmd/osgen (a separate Go module). Mostly comment additions and minor formatting; no behavior change. - Remove the dead pre-emit/ render pipeline: cmd/osgen/api_render.go (843 lines) cmd/osgen/api_render_plugin_client.go (207 lines) cmd/osgen/api_testgen.go (448 lines) cmd/osgen/api_testgen_integ.go (421 lines) cmd/osgen/render_integration_test.go (596 lines, the only consumer) These were superseded by cmd/osgen/emit/. The active code path runs through emit.Build; the deleted files were called only from the legacy integration test that goes with them. resolveFieldPath, the only function that survived as a live dependency from this set, is preserved next to subClientHierarchy in cmd/osgen/route.go. Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * Fix reqXxxReq concatenation in dispatch fragment template The dispatch template at cmd/osgen/emit/frag_dispatch.go used `{{- ""}}` followed by a `{{- if ...}}` to strip whitespace, which also stripped the space between "req" and the type name. The generator emitted "func (c Client) Bulk(ctx context.Context, reqBulkReq)" - a syntactically invalid parameter list - whenever IsPointerReq was false (value-receiver Req struct). Replace the multi-line whitespace-stripping construct with a single inline `{{if $.IsPointerReq}}*{{end}}` so the template emits "req X" for value Req and "req *X" for pointer Req correctly. The bug only surfaced when generating the value-receiver dispatch path (Bulk, Create, Delete, etc.); pointer-receiver Req types worked because the leading `*` masked the missing space. Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * Move osapi to v5preview/opensearchapi Relocate the generated v5-track API surface from `osapi/` to `v5preview/opensearchapi/` ahead of merging this PR. Two motivations: - Signal preview status: the `v5preview/` parent directory makes the package's stability guarantee explicit at the import path. Callers importing `.../v5preview/opensearchapi` cannot mistake it for the stable v5 surface. - Reserve the canonical name: the package is already named `opensearchapi` (matching the legacy hand-written package). Once v5 is cut and the legacy package is removed, the v5preview/ prefix gets dropped and consumers move with a single import-path edit; the type qualifiers (`opensearchapi.IndexReq`, etc.) stay constant. Generator changes that make the relocation a one-line config swap later: - cmd/osgen/ir/defaults.go (new): centralizes ModulePath, DefaultCorePkgName, DefaultCoreSubpath, DefaultCoreImportPath, and DefaultPluginsImportBase. Promoting v5preview/opensearchapi to the module root reduces to editing DefaultCoreSubpath. - cmd/osgen/emit/build.go: introduces coreImportPath() to centralize import-path resolution; importPathForGroup and the dispatch test emitter route through it. - Plugin test helper directory now uses `internal/<pkg>test` instead of `internal/test`, so the helper subpackage is unique per plugin (avoids collisions when multiple generated packages share the same module). Companion edits across CHANGELOG.md, UPGRADING.md, DEVELOPER_GUIDE.md, .codecov.yml, .github/workflows/check-gen.yml, .golangci.yml, Makefile, guides/security.md, and cmd/osgen/README.md update path references; the file moves themselves are pure git renames preserving history. The follow-up commit regenerates the package contents at the new location. Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * Regenerate osapi as v5preview/opensearchapi Pure regeneration output: `make gen` after the prior commit moved the package and updated cmd/osgen's defaults to emit v5preview/opensearchapi/. No semantic change. The 6042/6042 line balance reflects two mechanical substitutions across 1590 generated files: - `package osapi` -> `package opensearchapi` (and `osapi_test` -> `opensearchapi_test` for external test files). - Import path `github.com/opensearch-project/opensearch-go/v4/osapi` -> `.../v4/v5preview/opensearchapi`, with the qualifier `osapi.` -> `opensearchapi.` updated at every reference inside the regenerated test fixtures. No hand-edits in this commit; any review should focus on the prior move/codegen-config commit. Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * Fix data race in pooledConns Release-then-read test The "release clears pointer references across full capacity" subtest read the backing array after b.Release(), racing with parallel rendezvousTopK calls that re-acquired the same buffer from connSlicePool. Factor the clear-and-reset logic into a shared clearConns helper and test it directly; same invariant coverage with no use-after-Release. Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * Set ?timeout=10s on cat-nodes readiness probe clusterLensFSMCheck polls cat-nodes to advance LayerStatsReady. Without ?timeout=, cat-nodes' inner NodesInfo+NodesStats RPCs (server-side at RestNodesAction.java:128,140) inherit the unbounded request default. A freshly joined node whose first stats cycle stalls leaves the row with cpu=null, heap.percent=null indefinitely, and the readiness gate hangs until the per-layer budget elapses; CI run 26578837681 hit this on 2.13.0 insecure with TestNodesInfo + TestNodesStats consuming the full package timeout. 10s bounds each server-side fan-out and lets the Go-side polling cadence drive retries instead of relying on individual server-side calls to complete. Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> --------- Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> Co-authored-by: Jakob <jakob.hahn@hetzner.com> Co-authored-by: ssunno <ssunno@users.noreply.github.com>
sean-
force-pushed
the
gh-816
branch
13 times, most recently
from
June 1, 2026 14:58
3bf7aa9 to
c931cc8
Compare
sean-
force-pushed
the
gh-816
branch
2 times, most recently
from
June 2, 2026 17:52
81106c3 to
e22f6d9
Compare
…roject#851) Bumps [github.com/aws/aws-sdk-go-v2](https://github.com/aws/aws-sdk-go-v2) from 1.41.7 to 1.41.9. - [Release notes](https://github.com/aws/aws-sdk-go-v2/releases) - [Commits](aws/aws-sdk-go-v2@v1.41.7...v1.41.9) --- updated-dependencies: - dependency-name: github.com/aws/aws-sdk-go-v2 dependency-version: 1.41.9 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Upstream's merged opensearch-project#804 code-generation tests use raw "GET"/"POST" method strings. Convert the osgen emit/path test fixtures to the net/http http.Method* constants for type safety and consistency with the rest of the suite. The opensearch-project#804 codegen work itself is inherited from upstream; this commit carries only the test-fixture styling delta. Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com>
OpenSearch < 2.2.0 with the security plugin throws java.io.OptionalDataException on shard-routed requests due to non-thread-safe HashSet/HashMap in User serialization. Fixed in 2.2.0 by opensearch-project/security#1970 (50a94b47). Widen the existing 2.1.0-only skip to cover all versions below 2.2.0. Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com>
… write operations OpenSearch returns HTTP 200 for partial failures (bulk item errors, shard failures), forcing callers to double-check responses after err == nil. This adds PartialBulkError, PartialSearchError, and ShardFailureError types so callers only need the standard Go if err != nil idiom. Both (resp, err) are non-nil on partial failure — the response is fully populated. Gated behind Config.ReturnQueryErrors (default false in v4, will flip to true in v5). Existing behavior is unchanged unless opted in. Adds helper functions IsPartialFailure, ToleratePartialFailures, and RequireSuccessRate for threshold-based error tolerance. Ref: opensearch-project#816 Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com>
…rationID Add a zero-allocation HTTP method+path classifier that reuses the existing routeTrie to map requests to structured OperationID values. Enables transparent metrics, tracing, and access-control middleware at the http.RoundTripper layer without per-operation wrapper code. OperationID is a bit-packed int64 encoding R/W flag, category, and minor operation. Masking helpers (IsWrite, Category, Minor) support efficient bitwise filtering. String() returns Prometheus-friendly labels. OperationClassifier is built from the canonical route table and is safe for concurrent use. Returns OpOther for unrecognized patterns. Adds OperationID field to trieLeaf/trieMatch, OpID() to the Route interface, and .Op() to RouteBuilder. All 124 routes are tagged. Ref: opensearch-project#816 Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com>
Decouple calculation of jitter from runtime execution of backoff. Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com>
…coverNodesOnStart DiscoverNodes now waits for an in-flight discovery to complete (or for the context to be cancelled) instead of returning nil immediately. This lets callers block until topology data is available after client construction. - Auto-enable DiscoverNodesOnStart when OPENSEARCH_GO_ROUTER=true and the caller did not set the field Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com>
OpenSearch returns HTTP 200 for partial successes -- bulk item failures,
search-shard failures, single-doc replica failures -- so callers must
remember a second check after `err == nil`. v4 added an opt-in boolean
(Config.ReturnQueryErrors) that converted ALL partial-failure shapes
into typed Go errors. That single switch is too coarse: callers who
want shard-level errors but tolerate bulk item failures (or vice versa)
have no way to express it.
Replace the boolean with internal/errmask.ErrorMask, a 15-bit field
where each bit corresponds to one wrapper schema in the proposed
x-error-responses OpenAPI extension (BulkItems, SearchShards,
WriteShards, BroadcastShards, NodeFailures, BulkByScrollFailures,
TaskFailures, MultiSearchItems, MultiDocItems, Snapshot{Create,Get}-
ShardFailures, SimulateDocFailures, RankEvalFailures,
IngestionShardFailures, PitNodeFailures). A set bit MASKS that
category; the zero value reports every category. Callers express
fine-grained policy in code (Config.Errors = errmask.BulkItems |
errmask.SearchShards) or via OPENSEARCH_GO_ERROR_MASK using
comma-separated +/- tokens (e.g. "+all,-bulk_items").
Lifecycle (matches OPENSEARCH_GO_ROUTER):
v4 (this commit): default `errmask.All` -- preserves pre-bitfield
behavior (no partial-failure errors). Config.ReturnQueryErrors=true
is honored as a deprecated alias for `errmask.None`.
v5: default flips to `errmask.None` (safe by default).
v6: Config.Errors / OPENSEARCH_GO_ERROR_MASK removed; behavior is
unconditionally `errmask.None`.
The hand-written v4 opensearchapi/api_*.go call sites now read
c.errors.Has(errmask.<Wrapper>) for each operation's wrapper category.
A new hand-written v5preview/opensearchapi/errors.go ports the same
typed-error surface (PartialBulkError, PartialSearchError,
ShardFailureError, plus the IsPartialFailure / ToleratePartial-
Failures / RequireSuccessRate helpers) using v5preview's BulkResponse-
Item and ShardSearchFailure types. v5preview Config.Errors and the
clientInit(rootClient, mask) signature are wired through both
hand-written api.go and the generated clients_gen.go.
Spec side: opensearch-openapi.yaml is patched with 15
`_common.errors___<Wrapper>` schemas under components.schemas and 115
operation entries get an x-error-responses annotation. This mirrors
the upstream proposal in opensearch-api-specification (see
issue-x-partial-failure-mode.md). Once that PR lands and we re-bundle
from source, the local patch goes away cleanly.
Generator side: cmd/osgen reads x-error-responses from the spec
extension into ir.Operation.ErrorWrappers; cmd/osgen/errwrap supplies
a hardcoded fallback for plugin operations the spec doesn't yet
annotate. The dispatch fragment carries a data-driven `wrappers` map
of {Template, Applies}: each wrapper has both an emission template and
an Applies predicate that walks the response struct (including
embeds via the type registry) to confirm the field path the template
references actually exists. This keeps generated code compilable when
spec annotations land before the underlying response schema models the
relevant field -- v5preview's CreateResp and msearch's union response
item are skipped today and will start emitting once those types
acquire the missing fields.
Ref: opensearch-project#816
Ref: opensearch-project/opensearch-api-specification/pull/1137
Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com>
Mechanical output of cmd/osgen against the spec patched in the parent
commit. Two flavors of change:
1. clients_gen.go now declares `errors errmask.ErrorMask` on Client
and clientInit takes the mask as a second arg, matching the
hand-written api.go callers from the parent commit.
2. 19 operation dispatch files emit a per-wrapper post-do() block
that returns a typed error when the corresponding errmask bit is
unset and the wire data carries a partial failure. Wrappers
covered today: BulkItems, SearchShards, WriteShards,
MultiSearchItems. Wrappers in the catalog without hand-written
emission yet (BroadcastShards, NodeFailures, BulkByScrollFailures,
TaskFailures, MultiDocItems, Snapshot{Create,Get}ShardFailures,
SimulateDocFailures, RankEvalFailures, IngestionShardFailures,
PitNodeFailures) have their bits reserved by the spec annotation
but produce no generated check until detection logic is added in
a follow-up.
Operations whose typed response shape is missing the field path a
wrapper needs (CreateResp lacks _shards; msearch's union response
items lack a top-level Shards) are silently skipped by the dispatch
fragment's Applies guard. The annotation stays in the spec so the
emission will start automatically once the response types catch up.
Ref: opensearch-project#816
Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com>
…mission
Request-body subtree unions (e.g. ReindexSourceSort) were rendered as
`type Foo struct {}` because op.ReqBodySiblings fed SiblingTypesFragment
unconditionally, while op.SiblingTypes already split unions out and
routed them to UnionFragment. Apply the same partition to both lists
via a shared splitUnionsFromSiblings helper.
Plumb Op + Registry into UnionFragment so plugin-package unions
qualify cross-package branches as opensearchapi.FieldSort instead of
emitting bare names that fail to compile.
Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com>
Each generated discriminated union gains: - New<Union>From<Branch>(v <branch type>) <Union> per branch - SetRaw(json.RawMessage) typed escape hatch Lets callers populate request-body unions without dropping into BodyReader. SetRaw clears the typed branch so MarshalJSON returns the raw bytes verbatim. Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com>
Mechanical output of cmd/osgen against the patched spec, picking up: - ReindexSourceSort (and other request-body subtree unions) now emit as proper discriminated unions instead of empty structs - Plugin-package unions qualify cross-package branches with the opensearchapi. prefix - Every union gains New<Union>From<Branch> constructors and a SetRaw(json.RawMessage) escape hatch Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com>
…tor race OpenSearch versions <=2.17.x carry a node-join/node-left race condition in the cluster coordinator that leaves a node visible in cluster state but disconnected at the transport layer. The cluster-manager's NodesStats RPC fan-out then fails fast against the dead connection cache, so /_cat/nodes returns rows with cpu=null and heap.percent=null indefinitely for the affected node. Any readiness gate that polls /_cat/nodes (such as ours, advancing nodes to LayerStatsReady) sits at the per-layer budget without progressing, until the package-wide go test -timeout fires and panics. The bug is fixed in 2.18+ and 3.x via opensearch-project/OpenSearch#15521 ("Fix for race condition in node-join/node-left loop"), backported to the 2.x line via opensearch-project/OpenSearch#16118. No setting on older versions avoids the race; the disconnect ordering is structural to the coordinator's task queue handling. Single-node clusters cannot hit the race because there is no peer to coordinate join/left tasks across. For affected versions, run with OPENSEARCH_NODE_COUNT=1 so the cluster-manager has nothing to fan stats RPCs out to. The test suite still exercises the same client surface; the only thing skipped is multi-node-specific server behavior, which opensearch-go itself does not implement. Changes: - .github/workflows/test-compatibility.yml: per-entry node_count matrix field. 1 for 1.3.20 through 2.17.1; 3 for 2.18.0 and later (including main/latest). Matrix comment cites the upstream PRs so the cutoff is greppable. - Makefile (cluster.docker-up): respect OPENSEARCH_NODE_COUNT and pass --scale opensearch-nodeN=0 flags to docker compose accordingly. Defaults to 3 for local development. When 2.18+ is the new floor for client compatibility, the per-entry overrides can be removed and OPENSEARCH_NODE_COUNT can return to a fixed env var. Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com>
…v opt-out v5preview/opensearchapi.NewClient and NewDefaultClient now inject opensearchtransport.NewDefaultRouter when config.Client.Router is nil, opting every v5preview client into intelligent request routing (role-aware dispatch, RTT-based scoring, congestion-window AIMD, shard-cost weighting) by default. The OPENSEARCH_GO_ROUTER environment variable acts as a symmetric override: setting it to a falsy value (false/0) suppresses the injection so Router stays nil, matching v4's "no router" behavior without code changes. Unset, truthy, and unparseable values all proceed with injection. Caller-provided Routers are preserved unchanged. The only behavioral divergence from v4's NewClient is the env-unset row: v4 leaves Router alone, v5preview injects the default. Truthy and falsy values keep their existing semantics. internal/envvars: add Falsy(name) helper. Truthy returns false for both "unset" and "explicitly opted out"; Falsy distinguishes them so the v5preview rule can be expressed as `!envvars.Falsy(envvars.Router)` without re-implementing the parse logic. Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com>
…ic Go names The codegen rewrites non-idiomatic substrings produced by pascal-casing spec names into idiomatic Go forms: Msearch -> MSearch (multi-search initialism) Mget -> MGet (multi-get initialism) Mtermvectors -> MTermVectors (initialism + compound noun) Termvectors -> TermVectors (compound noun split) Forcemerge -> ForceMerge (compound verb split) Response -> Resp (idiomatic short form) Each rule matches at PascalCase boundaries; lowercase-suffix variants (Responses, Responsible, ...) are preserved. The Response -> Resp rule additionally requires the trailing character to be uppercase, so standalone names like SearchResponse (spec wrapper) don't collide with the operation-level <Op>Resp response-body name. frag_dispatch.go: the BulkItems wrapper template now resolves its walked element type from the IR (via bulkInnerItemType, which walks Items[] -> outer wrapper -> first pointer field -> inner type) instead of hardcoding "BulkRespItem". Future spec or naming changes propagate automatically. Renames applied to hand-written types for v4/v5 consistency: BulkResponseItem -> BulkRespItem, ErrorResponseBase -> ErrorRespBase, MsearchErrors -> MSearchErrors, MsearchTemplateErrors -> MSearchTemplateErrors. v4's hand-written errors.go and dispatch sites updated to match. Refs opensearch-project#816 Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com>
Mechanical output of cmd/osgen against the patched spec, picking up the idiomatic abbreviations from the parent commit. Touches every generated type that contained Msearch, Mget, Mtermvectors, Termvectors, Forcemerge, or a compound *Response* substring. Refs opensearch-project#816 Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com>
…for aggregations
Two classes of try-each discriminated union are replaced with single-pass
strategies. On mget this cuts decode allocations ~2.7x (1000 docs: ~43k -> ~16k
allocs/op) and time ~1.7x (4.2ms -> 2.5ms); the remaining cost is the GetResult
decode itself plus interface boxing, not the union machinery.
- Case A (merged): object unions with one permissive "primary" branch plus
discriminated branch(es) -- mget, msearch, indices-open. The primary is
embedded and the common case decodes in a single json.Unmarshal; each
discriminated branch is detected by the presence of its distinguishing key and
decoded only when matched. This drops the build.HasJSONKeys map probe (which
was ~61% of the old allocations) and the per-item raw copy.
- Case B (lazy As<T>()): aggregation/suggest result unions carry no wire
discriminator (avg/sum/min/max all serialize as {"value":N}, and bucket types
collide), so they cannot be auto-selected. UnmarshalJSON only retains the raw
bytes; generated As<ConcreteType>() accessors decode on demand into the type
the caller requested.
- Unions fitting neither (e.g. reindex bodies, plugin-defined task status) keep
the existing try-each decoder; the classifier logs once per union name when it
declines to convert a wrapper-shaped union.
- All union UnmarshalJSON now aliases the owned response buffer (u.raw = data)
rather than copying it; RawJSON() documents the borrowed-buffer contract
(valid while the response is reachable, copy to retain).
Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com>
Regenerated output of the osgen union changes: merged single-pass decoders for mget/msearch/indices-open success|error items, As<T>() accessors for aggregation and suggest result unions, and buffer-aliasing UnmarshalJSON across all unions. Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com>
Add integration tests that drive real requests against the cluster and assert the decoded shape of each union branch, covering the merged single-pass decode and lazy As<T>() paths end-to-end per server version: - aggregation: lazy As<T>() accessors (terms, date_histogram, stats, avg, sum, min, max, value_count, cardinality) - mget: merged success|error decode (GetResult found/not-found vs MGetMultiGetError) - msearch: merged success|error decode (MSearchMultiSearchItem vs ErrorRespBase), the first-byte-switch SearchHitsMetadataTotal union, and the MultiSearchItemError partial-failure surface Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com>
- github.com/aws/aws-sdk-go-v2/config from 1.32.18 to 1.32.20 - github.com/aws/aws-sdk-go-v2/credentials from 1.19.17 to 1.19.19 - github.com/getkin/kin-openapi from v0.139.0 to v0.140.0 Fixes: opensearch-project#852, opensearch-project#850 Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com>
v5preview/opensearchapi/README.md covers Partial Failure Errors (Config.Errors, errmask, OPENSEARCH_GO_ERROR_MASK, typed errors, opensearchapi.Errors helper, per-Resp helpers, helper functions, operation constants) and Default Router Injection (truth table for OPENSEARCH_GO_ROUTER, opt-out semantics). v5preview/opensearchapi/MIGRATING.md captures the v4 -> v5preview surface delta: import path, Indices -> Index on multi-index Req types, DocumentID -> ID on IndexReq, optional Params becomes *Params, optional bool query params become *bool, partial-failure type renames, errmask default flip, default Router injection. Framed as a one-time cost of adopting the spec-generated client. guides/error_handling.md presents Bulk, Search, and Write partial- failure examples as paired v4/v5preview blocks. New Per-Resp helper subsection covers BulkItemFailures, SearchShardFailures, WriteShardFailures, MultiSearchItemFailures, PartialFailures(mask). Error Type Reference table includes MultiSearchItemError, MSearchErrors, MSearchTemplateErrors and accurately describes the v4 vs v5preview internal-field-type divergence. guides/bulk.md notes the v4/v5preview field-name and BulkResp.Items shape divergences up front, with paired error-iteration examples for v4's []map[string]BulkRespItem and v5preview's []BulkItem. UPGRADING.md keeps the version-history essentials for the >=5.0 partial-failure model and v5preview Router injection and forward-links to the new package docs. Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com>
sean-
added a commit
that referenced
this pull request
Jul 13, 2026
…trics with lock-free structures (opensearch-project#901) * feat(opensearchtransport)!: remove EnableMetrics, make detailed metrics always-on and lock-free (opensearch-project#892) Remove EnableMetrics from the client and transport Config. Detailed metrics are now always collected; Metrics() always returns the full snapshot. (BREAKING) Convert deadSince/overloadedAt from mu-guarded time.Time to lock-free atomic.Int64 (UnixNano, 0 = unset). Writes still occur under c.mu so the resurrection/standby read-modify-write decisions stay serialized; only the reads went lock-free, so buildConnectionMetric no longer takes each connection's mutex. This eliminates the #1 explicit-lock contention site measured under concurrent Perform() load. Signed-off-by: Ryan Yuan <ryan.yuan@crowdstrike.com> * docs(opensearchtransport): disambiguate uninitialized-metrics error The defensive nil-metrics path returned "transport metrics not enabled", recycling the removed EnableMetrics concept and contradicting the Metrics() godoc that the error is non-nil only on snapshot-callback failure. Reword the error to "transport metrics not initialized", state both error conditions in the godoc, and align the test comment. Signed-off-by: Ryan Yuan <ryan.yuan@crowdstrike.com> * test(opensearchtransport): drop EnableMetrics references from comments The test comments narrated the removed EnableMetrics flag as history. Describe the end-state behavior instead: callbacks register when a router has policies, and the detailed snapshot runs unconditionally. Signed-off-by: Ryan Yuan <ryan.yuan@crowdstrike.com> * fix(opensearchtransport): lock pool write in RolePolicy.DiscoveryUpdate RolePolicy.DiscoveryUpdate called recalculateWarmupParams without holding the pool write lock, while the roundrobin and cluster_coordinator policies took pool.Lock() for the identical call. recalculateWarmupParams writes the pool's warmupRounds, warmupSkipCount, and activeListCap fields, which getWarmupParams and the other DiscoveryUpdate callers read and write under that same lock. Two concurrent DiscoverNodes calls on a shared transport therefore raced on those fields (observed in CI: two goroutines writing in recalculateWarmupParams via RolePolicy.DiscoveryUpdate). This is pre-existing on main, unrelated to the EnableMetrics removal. Compute the projected pool size and recalculate the warmup parameters under pool.Lock(), releasing before discoveryUpdateAdd/Remove (which acquire the lock per-connection). Add TestRolePolicyDiscoveryUpdateConcurrent, which reproduces the race under -race and passes with the fix. Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> --------- Signed-off-by: Ryan Yuan <ryan.yuan@crowdstrike.com> Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> Co-authored-by: Sean Chittenden <sean.chittenden@crowdstrike.com>
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
Adds two features that make the Go client more useful for production consumers:
Partial failure errors in
opensearchapi/—PartialBulkError,PartialSearchError, andShardFailureErrortypes returned from API methods whenConfig.ReturnQueryErrorsis set, so callers only needif err != nil. Both(resp, err)are non-nil on partial failure — the response is fully populated. Controllable viaOPENSEARCH_GO_PARTIAL_QUERY_ERRORSenv var (overrides config) for deploy-time toggling.OperationClassifierinopensearchtransport/— A bit-packedOperationIDtype and exported classifier that maps any HTTP method+path to a structured operation identifier. Enables transparent metrics, tracing, and access-control middleware at thehttp.RoundTripperlayer without per-operation wrapper code.OPENSEARCH_GO_ROUTERenv var — Enable the DefaultRouter (connection-scoring, role-aware routing) without code changes by settingOPENSEARCH_GO_ROUTER=true. ProgrammaticConfig.Routertakes precedence. Transitional: off by default in v4, on by default in v5, removed in v6.All three features are additive and backward-compatible. Existing code is unaffected unless the caller opts in to
ReturnQueryErrorsorOPENSEARCH_GO_ROUTER=true.Ref: opensearch-project#816
Motivation
Partial failures require two error checks
OpenSearch returns HTTP 200 for partial failures — bulk item errors, shard failures on search, replica failures on writes. This forces non-idiomatic double-checking:
Go convention (
io.Reader.Read,json.Decoder.Decode) is to return(result, error)where both can be non-nil. The caller should only needif err != nil.No way to identify the operation from an HTTP request
Middleware that wraps
http.RoundTripper(metrics, tracing, audit logging) needs to know which OpenSearch operation a request represents. Today the only option is to re-parse the URL path with hand-written regexes, which is fragile and incomplete. The transport already has a trie-based route matcher. Exposing a read-only classifier that reuses this trie gives middleware a zero-allocation, O(path-segments) lookup that stays in sync with the canonical route table.Changes
opensearchapi/— Partial failure errorserrors.go(new)PartialFailureErrorinterface,PartialBulkError,PartialSearchError,ShardFailureError, operation constants, helperserrors_test.go(new)_testpackage): interface compliance,errors.As, all helpers, edge caseserrors_internal_test.go(new)resolveReturnQueryErrorsenv var priority (env > config > default),strconv.ParseBooledge casesopensearchapi.goConfig.ReturnQueryErrors,resolveReturnQueryErrors()withOPENSEARCH_GO_PARTIAL_QUERY_ERRORSenv var, updated constructorsapi_bulk.goPartialBulkErrorwhendata.Errors && returnQueryErrorsapi_search.goPartialSearchErrorwhenShards.Failed > 0 && returnQueryErrorsapi_index.goShardFailureErrorwhenShards.Failed > 0 && returnQueryErrorsapi_document.goShardFailureErrorfor Create and Deleteapi_update.goShardFailureErrorwhenShards.Failed > 0 && returnQueryErrorsapi_scroll.goPartialSearchErrorfor Scroll.Getapi_search-template.goPartialSearchErrorwhenShards.Failed > 0 && returnQueryErrorsapi_msearch.goPartialSearchErrorwith aggregated shard failures across sub-responsesapi_msearch-template.goPartialSearchErrorwith aggregated shard failures across sub-responsesopensearchtransport/— Operation classifieroperation.go(new)OperationIDtype, bit layout, categories, minors, ~200 composedOpconstants,String()classify.go(new)OperationClassifier,NewOperationClassifier(),Classify()classify_test.go(new)String(), concurrent safetypolicy_mux_trie.gooperationID OperationIDtotrieLeafandtrieMatchpolicy_mux.goOpID()toRouteinterface,Op()builder method onRouteBuilderpolicy_mux_trie_test.goadd()call signaturespolicy_mux_internal_test.goOpID()on mock route typerouter.go.Op(OpXxx)on all ~124 routes inbuildRoleRoutes()feature_config.goenvRouterconstant with v4/v5/v6 lifecycle doc commentopensearchtransport.goNewDefaultRouter()inNew()when env var is truthy and no programmatic router setrouter_coverage_internal_test.godoc.goDocumentation
CHANGELOG.mdUPGRADING.mdReturnQueryErrorsdefault flipUSER_GUIDE.mdReturnQueryErrorsin example config, env var table, Operation Classifier sectionguides/error_handling.mdguides/routing.mdUPGRADING.mdReturnQueryErrorsdefault flip; router env var with v6 removal noteUSER_GUIDE.mdReturnQueryErrorsin example config, env var table (includingOPENSEARCH_GO_ROUTER), Operation Classifier sectionMigration path
ReturnQueryErrorsdefaults tofalse. Users opt in viaConfigorOPENSEARCH_GO_PARTIAL_QUERY_ERRORS=true. Zero breaking changes.Config.Router = NewDefaultRouter()orOPENSEARCH_GO_ROUTER=true.true. Users who need the old behavior setReturnQueryErrors: falseorOPENSEARCH_GO_PARTIAL_QUERY_ERRORS=false.OPENSEARCH_GO_ROUTER=false.OPENSEARCH_GO_ROUTERenv var removed. Router is unconditionally on; useOPENSEARCH_GO_POLICY_*to disable individual policies.OPENSEARCH_GO_ROUTERenv var removed. Router is unconditionally on; useOPENSEARCH_GO_POLICY_*to disable individual policies.