Do I need to `complete()` takeUntil Subject inside ngOnDestroy?

Short answer, no this is not needed, but it also doesn’t hurt.

Long answer:

Unsubscribing / completing in angular is only needed when it prevents garbage collection because the subscription involves some subject that will outlive the component due to be collected. This is how a memory leak is created.

if I have service:

export class MyService {
  mySubject = new Subject();
}

which is provided in root and only root (which means it’s a singleton and will never be destroyed after instantiation) and a component that injects this service and subscribes to it’s subject

export class MyLeakyComponent {
  constructor(private myService: MyService) {
    this.myService.mySubject.subscribe(v => console.log(v));
  }
}

this is creating a memory leak. Why? because the subscription in MyLeakyComponent is referenced by the subject in MyService, so MyLeakyComponent can’t be garbage collected so long as MyService exists and holds a reference to it, and MyService will exist for the life of the application. This compounds everytime you instantiate MyLeakyComponent. To fix this, you must either unsubscribe or add a terminating operator in the component.

however this component:

export class MySafeComponent {
  private mySubect = new Subject();
  constructor() {
    this.mySubject.subscribe(v => console.log(v));
  }
}

is completely safe and will be garbage collected without issue. No external persisting entity holds a reference to it. This is also safe:

@Component({
  providers: [MyService]
})
export class MyNotLeakyComponent {
  constructor(private myService: MyService) {
    this.myService.mySubject.subscribe(v => console.log(v));
  }
}

now the injected service is provided by the component, so the service and the component will be destroyed together and can be safely garbage collected as the external reference will be destroyed as well.

Also this is safe:

export class MyHttpService { // root provided
  constructor(private http: HttpClient) {}
  
  makeHttpCall() {
    return this.http.get('google.com');
  }
}

export class MyHttpComponent {
  constructor(private myhttpService: MyHttpService) {
    this.myhttpService.makeHttpCall().subscribe(v => console.log(v));
  }
}

because http calls are a class of observables that are self terminating, so they terminate naturally after the call completes, so no need to manually complete or unsubscribe, as the external reference is gone once it naturally completes.

As to your example:
the unsubscribe$ subject is local to the component, thus it cannot possibly cause a memory leak. This is true of any local subject.

A note on best practices:
Observables are COMPLEX. One that might look completely safe, could involve an external subject in a subtle manner. To be totally safe / if you’re not extremely comfortable with observables, it is generally recommended that you unsubscribe from all non terminating observables. There isn’t a downside other than your own time spent doing it. I personally find the unsubscribe$ signal method hacky and think it pollutes / confuses your streams. the easiest to me is something like this:

export class MyCleanedComponent implements OnDestroy {
  private subs: Subscription[] = [];
  constructor(private myService: MyService) {
    this.subs.push(
      this.myService.mySubject.subscribe(v => console.log(v)),
      this.myService.mySubject1.subscribe(v => console.log(v)),
      this.myService.mySubject2.subscribe(v => console.log(v))
    );
  }
  
  ngOnDestroy() {
    this.subs.forEach(s => s.unsubscribe());
  }
}

However, the single BEST method for preventing leaks is using the async pipe provided by angular as much as possible. It handles all subscription management for you.