Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Disable tracing in integration tests #31308

Closed
mhalbritter opened this issue Jun 9, 2022 · 3 comments
Closed

Disable tracing in integration tests #31308

mhalbritter opened this issue Jun 9, 2022 · 3 comments
Labels
type: enhancement A general enhancement
Milestone

Comments

@mhalbritter
Copy link
Contributor

Metrics are by default disabled in integration tests, you have to enable them explicitly with @AutoConfigureMetrics. The same is not true for tracing at the moment. We should disable tracing in integration tests, too. Think about if we should rename the annotation to @AutoConfigureObservability which would enable metrics and tracing in the integration tests.

@mhalbritter mhalbritter added the type: enhancement A general enhancement label Jun 9, 2022
@mhalbritter mhalbritter added this to the 3.0.x milestone Jun 9, 2022
@wilkinsona
Copy link
Member

I like the renaming idea. I wonder if a user would ever want to only enable metrics or only enable tracing? If so, perhaps the annotation could have an attribute that allows you to control what's enabled.

Something like this (but with better names):

public @interface AutoConfigureObservability {

	Component[] value() default { Component.METRICS, Component.TRACING };

	enum Component {

		METRICS, TRACING;

	}

}

Or, given that observability's unlikely to expand much, if at all, in terms of the number of components, it could be two booleans:

public @interface AutoConfigureObservability {

	boolean metrics() default true;

	boolean tracing() default true;

}

We could keep @AutoConfigureMetrics in a deprecated form for backwards compatibility "delegating" to @AutoConfigureObservability:

@AutoConfigureObservability(tracing = false)
public @interface AutoConfigureMetrics {

}

@mhalbritter
Copy link
Contributor Author

I really like this, thanks Andy!

mhalbritter added a commit that referenced this issue Jun 10, 2022
This annotation is read by ObservabilityContextCustomizerFactory, which
then sets test properties depending on the annotation attributes.

@AutoConfigureMetrics is deprecated, to support backwards compatability
it's now meta-annotated with @AutoConfigureObservability

See gh-31308
@mhalbritter
Copy link
Contributor Author

I've implemented the @AutoConfigureObservability annotation, deprecated the @AutoConfigureMetrics one, updated the docs, and all our tracing auto-configurations now back off in tests. I will add the deprecation of @AutoConfigureMetrics to the release notes.

@mhalbritter mhalbritter modified the milestones: 3.0.x, 3.0.0-M4 Jun 10, 2022
ikhoon added a commit to line/armeria that referenced this issue Jan 25, 2024
Motivation:

Spring 6 HTTP client is split into two parts.
#4838 (comment) It
will be easier to review if Spring dependencies are upgraded before
working on Spring HTTP 6 integration.

Dependencies:
- Spring 6.0.13 -> 6.1.2
- Spring Boot 2.7.16 -> 2.7.18, 3.1.4 -> 3.2.1
- Jetty 12.0.5

Modifications:

- Spring Boot Actuator
- `@AutoConfigureMetrics` has been removed in favor of
`@AutoConfigureObservability`.
spring-projects/spring-boot#31308
- `@EnableTestMetrics` is added in Spring Boot 2 and 3 modules to share
the same test code.
- `@EnableTestMetrics` in `boot2-actuator-autoconfigure` inherits
`@AutoConfigureMetrics` but `@AutoConfigureObservability` is inherited
in `boot3-actuator-autoconfigure`.
- Spring Boot WebFlux
- `AbstractServerHttpRequestVersionSpecific` is added to shim the API
changes in Spring 6.1
- Some return types are wrapped in `Mono` in test REST APIs. Otherwise,
the service method is invoked with a blocking executor of Spring
WebFlux.
spring-projects/spring-framework@b016f38#diff-8fd6b8e3408492ba1cabbbea613ac33b7c674db0a9287a2d479e83ec55c1544eR253-R254
- Jetty 12
- Jetty version has been upgraded to 12 in Spring Boot 3.2.
spring-projects/spring-boot#36073
- Jetty core module is now independent of Servlet API.
https://webtide.com/introducing-jetty-12/
- Due to the change, the existing `JettyService` code can no longer be
reused, so `JettyService` is newly implemented based on the Jetty 12
API.
  - `JettyServiceBuilder`
- `JettyServiceBuilder.tlsReverseDnsLookup()` is removed because Jetty
`ServletApiRequest.getRemoteHost()` does not perform a reverse DNS
lookup.
- `JettyServiceBuilder.handlerWrapper(HandlerWrapper)` has been removed
in favor of `JettyServiceBuilder.insertHandler(Handler.Singleton)`
      - `HandlerWrapper` does not exist in Jetty 12.
- `insertHandler(Handler.Singleton)` is the most similar replacement.
- `JettyServiceBuilder.sessionIdManager(SessionIdManager)` and
`sessionIdManagerFactory(Function)` have been removed because
`Server.setSessionIdManager()` does not exist.
- Instead, `SessionIdManager` can be set via
`JettyServiceBuilder.bean()`
https://eclipse.dev/jetty/documentation/jetty-12/programming-guide/index.html#pg-server-session-idmgr
  - `JettyService`
- Jetty `Request` becomes immutable so Armeria request headers are
indirectly set via `HttpChannel.onRequest()`
- `HttpStreamOverHTTP1` provides a request body instead of
`HttpChannelOverHttp`.
- A response is now written to `HttpStreamOverHTTP1.send(...)`.
Previously, `HttpConnection.send(...)` was used.
- Failed to find the counterpart of `jReq.setDispatcherType()` and
`jReq.setAsyncSupported()`.
- Without `setAsyncSupported`, async could support via `Callback`
interface.
- `httpChannel.handle()` has been replaced with
`httpChannel.onRequest(requestMetadata).run()`.
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

2 participants