Skip to content

Commit

Permalink
refactor sidecar/interface.rs into smaller files (#395)
Browse files Browse the repository at this point in the history
Refactor the sidecar::interface file into multiple files in a sidecar::service namespace.

* remove unnecessary clippy allow for needless collect in sidecar
interface

MSRV is greater than 1.60, so we can remove this allow.

* refactor Sidecar::Interface::InstanceId to separate file

Also, add basic test and doc comments

* refactor Sidecar::Interface::QueueId to a separate file.

And add tests and doc comments

* refactor sidecar::interface::RuntimeMeta to separate file

And rename to RuntimeMetadata, add basic doc comments and tests.

* minor cleanup of sidecar::interface file.

* Fix typo for intitial_acitons input paramater to get_app
* Reorder impl to match trait order
* remove redundant prefixes

* refactor Sidecar::Interface::SerializedTracerHeaderTags to a separate file

Also, The From SerializedTracerHeaderTags and From TracerHeaderTags trait
impls were changed to try_into trait impls as it is possible (however
unlikely) that the code within the the trait impls could return errors
and it is preferable to let the caller decide how to handle those errors
rather than unwrap a Result and potentially panic.

* refactor RequestIdentifier and RequqestIdentification to separate file

and add doc comments

* refactor sidecar_interface from interface.rs to separate file

* Refactor sidecar::interface::SessionInfo to its own file.

Also, add rustdoc comments and tests. Uncovered a bug in the
shtudown_running_instances function where it never shutdown the running
instances.

* refactor Sidecar::SidecarServer to separate file

* extract session interceptor into separate function in sidecar_server

* sidecar server - move logic for processing of interceptor response

Refactor into separate function. Also, replace unwraps() with expects().

* Add rustdoc comments for public sidecar_server methods

* Minor tweak to sidecar session_info::shutdown_runtime to reduce mutex lock scope

* refactor sidecar RuntimeInfo from interface.rs to separate file

* refactor complex type in sidecar RuntimeInfo

* Sidecar - move SessionConfig, SidecarAction, and ApporQueue structs

Moving from interface.rs to separate files

* refactor sidecar::AppInstance in to separate file

* sidecar - refactor enqueued_telemetry_data and enqueued_telemetry_stats to separate files

* sidecar - refactor tracing logic in interface.rs to separate files

* refactor SidecarStats to telemetry namespace

* Move blocking sidecar_interface to separate file

* Reduce visibility scope of types after sidecar::interface refactor

interface.rs has been refactored in to a "service" module within
sidecar. During refactor the access level of some functions and types
was increased while they were being moved around. This should "fix" the
access level to the most restrictive possible.

* Move SidecarStats and AppOrQueue structs to more appropriate locations.

After refactor of sidecar::interface.rs into multiple files AppOrQueue belongs in the telemetry namespace and SidecarStats belongs closer to the code it is generating stats for in SidecarServer.
  • Loading branch information
ekump authored May 8, 2024
1 parent 5a69bdb commit d3f8544
Show file tree
Hide file tree
Showing 23 changed files with 2,724 additions and 1,664 deletions.
79 changes: 45 additions & 34 deletions sidecar-ffi/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,22 @@ use datadog_sidecar::agent_remote_config::{
};
use datadog_sidecar::config;
use datadog_sidecar::config::LogMethod;
use datadog_sidecar::one_way_shared_memory::{OneWayShmReader, ReaderOpener};
use datadog_sidecar::service::{
blocking::{self, SidecarTransport},
InstanceId, QueueId, RuntimeMetadata, SerializedTracerHeaderTags, SessionConfig, SidecarAction,
};
use ddcommon::Endpoint;
use ddcommon_ffi as ffi;
use ddcommon_ffi::MaybeError;
use ddtelemetry::{
data::{self, Dependency, Integration},
worker::{LifecycleAction, TelemetryActions},
};
use ddtelemetry_ffi::try_c;
use ffi::slice::AsBytes;
use libc::c_char;
use std::convert::TryInto;
use std::ffi::c_void;
use std::fs::File;
#[cfg(unix)]
Expand All @@ -20,21 +34,6 @@ use std::os::windows::io::{FromRawHandle, RawHandle};
use std::slice;
use std::time::Duration;

use datadog_sidecar::interface::{
blocking::{self, SidecarTransport},
InstanceId, QueueId, RuntimeMeta, SerializedTracerHeaderTags, SessionConfig, SidecarAction,
};
use datadog_sidecar::one_way_shared_memory::{OneWayShmReader, ReaderOpener};
use ddcommon::Endpoint;
use ddtelemetry::{
data::{self, Dependency, Integration},
worker::{LifecycleAction, TelemetryActions},
};
use ffi::slice::AsBytes;

use ddcommon_ffi::MaybeError;
use ddtelemetry_ffi::try_c;

#[repr(C)]
pub struct NativeFile {
pub handle: Box<PlatformHandle<File>>,
Expand Down Expand Up @@ -241,8 +240,8 @@ pub unsafe extern "C" fn ddog_sidecar_runtimeMeta_build(
language_name: ffi::CharSlice,
language_version: ffi::CharSlice,
tracer_version: ffi::CharSlice,
) -> Box<RuntimeMeta> {
let inner = RuntimeMeta::new(
) -> Box<RuntimeMetadata> {
let inner = RuntimeMetadata::new(
language_name.to_utf8_lossy(),
language_version.to_utf8_lossy(),
tracer_version.to_utf8_lossy(),
Expand All @@ -253,7 +252,7 @@ pub unsafe extern "C" fn ddog_sidecar_runtimeMeta_build(

#[no_mangle]
#[allow(clippy::missing_safety_doc)]
pub unsafe extern "C" fn ddog_sidecar_runtimeMeta_drop(meta: Box<RuntimeMeta>) {
pub unsafe extern "C" fn ddog_sidecar_runtimeMeta_drop(meta: Box<RuntimeMetadata>) {
drop(meta)
}

Expand Down Expand Up @@ -345,7 +344,7 @@ pub unsafe extern "C" fn ddog_sidecar_telemetry_flushServiceData(
transport: &mut Box<SidecarTransport>,
instance_id: &InstanceId,
queue_id: &QueueId,
runtime_meta: &RuntimeMeta,
runtime_meta: &RuntimeMetadata,
service_name: ffi::CharSlice,
env_name: ffi::CharSlice,
) -> MaybeError {
Expand Down Expand Up @@ -429,19 +428,27 @@ pub struct TracerHeaderTags<'a> {
pub client_computed_stats: bool,
}

impl<'a> From<&'a TracerHeaderTags<'a>> for SerializedTracerHeaderTags {
fn from(tags: &'a TracerHeaderTags<'a>) -> Self {
datadog_trace_utils::trace_utils::TracerHeaderTags {
lang: &tags.lang.to_utf8_lossy(),
lang_version: &tags.lang_version.to_utf8_lossy(),
lang_interpreter: &tags.lang_interpreter.to_utf8_lossy(),
lang_vendor: &tags.lang_vendor.to_utf8_lossy(),
tracer_version: &tags.tracer_version.to_utf8_lossy(),
container_id: &tags.container_id.to_utf8_lossy(),
client_computed_top_level: tags.client_computed_top_level,
client_computed_stats: tags.client_computed_stats,
}
.into()
impl<'a> TryInto<SerializedTracerHeaderTags> for &'a TracerHeaderTags<'a> {
type Error = std::io::Error;

fn try_into(self) -> Result<SerializedTracerHeaderTags, Self::Error> {
let tags = datadog_trace_utils::trace_utils::TracerHeaderTags {
lang: &self.lang.to_utf8_lossy(),
lang_version: &self.lang_version.to_utf8_lossy(),
lang_interpreter: &self.lang_interpreter.to_utf8_lossy(),
lang_vendor: &self.lang_vendor.to_utf8_lossy(),
tracer_version: &self.tracer_version.to_utf8_lossy(),
container_id: &self.container_id.to_utf8_lossy(),
client_computed_top_level: self.client_computed_top_level,
client_computed_stats: self.client_computed_stats,
};

tags.try_into().map_err(|_| {
std::io::Error::new(
std::io::ErrorKind::InvalidData,
"Failed to convert TracerHeaderTags to SerializedTracerHeaderTags",
)
})
}
}

Expand All @@ -453,11 +460,13 @@ pub unsafe extern "C" fn ddog_sidecar_send_trace_v04_shm(
shm_handle: Box<ShmHandle>,
tracer_header_tags: &TracerHeaderTags,
) -> MaybeError {
let tracer_header_tags = try_c!(tracer_header_tags.try_into());

try_c!(blocking::send_trace_v04_shm(
transport,
instance_id,
*shm_handle,
tracer_header_tags.into(),
tracer_header_tags,
));

MaybeError::None
Expand All @@ -471,11 +480,13 @@ pub unsafe extern "C" fn ddog_sidecar_send_trace_v04_bytes(
data: ffi::CharSlice,
tracer_header_tags: &TracerHeaderTags,
) -> MaybeError {
let tracer_header_tags = try_c!(tracer_header_tags.try_into());

try_c!(blocking::send_trace_v04_bytes(
transport,
instance_id,
data.as_bytes().to_vec(),
tracer_header_tags.into(),
tracer_header_tags,
));

MaybeError::None
Expand Down
4 changes: 2 additions & 2 deletions sidecar/src/entry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ use std::{
};
use tokio::sync::mpsc;

use crate::interface::blocking::SidecarTransport;
use crate::interface::SidecarServer;
use crate::service::blocking::SidecarTransport;
use crate::service::SidecarServer;
use datadog_ipc::platform::AsyncChannel;

use crate::setup::{self, IpcClient, IpcServer, Liaison};
Expand Down
Loading

0 comments on commit d3f8544

Please sign in to comment.